Acceptable/efficient syntax

[AdrianD]

I made a small change to remove max lvl 1 starting skills in client.cpp.

It appears to work after logging in but, I want to be certain the syntax is correct and if it could be done better.

My comments inside:

void Client::SetClassStartingSkills(PlayerProfile_Struc t *pp)
{
for (uint32 i = 0; i <= HIGHEST_SKILL; ++i) {
if (pp->skills[i] == 0); /* added ; just before this comment commented out 9-22-15 to test starting skills (I do not want max skills at lvl 1)
{
// Skip specialized, tradeskills (fishing excluded), Alcohol Tolerance, and Bind Wound
if (EQEmu::IsSpecializedSkill((SkillUseTypes)i) ||
(EQEmu::IsTradeskill((SkillUseTypes)i) && i != SkillFishing) ||
i == SkillAlcoholTolerance || i == SkillBindWound)
continue;

pp->skills[i] = database.GetSkillCap(pp->class_, (SkillUseTypes)i, 1);
} */
}
}

Thank you.

EDIT: I notice the client shows skills with values > 0 but the DB does not. I suppose the addition of quite a few more lines of code will be needed to account for the removal of the segment above to make accurate.

If this doesn't belong here please move it, thanks.

[Shendare]

The client will often display hard-coded level 1 starting stats no matter what you code into the server and enter into the database. I haven't tested to see if it corrects to what the server tells it at level two.

I'd say the very easiest thing to do in that method would simply be to add a return statement after the very first opening brace. Hehe.


void Client::SetClassStartingSkills(PlayerProfile_Struc t *pp)
{
return;
for (uint32 i = 0; i <= HIGHEST_SKILL; ++i) {
if (pp->skills[i] == 0) {
// Skip specialized, tradeskills (fishing excluded), Alcohol Tolerance, and Bind Wound
if (EQEmu::IsSpecializedSkill((SkillUseTypes)i) ||
(EQEmu::IsTradeskill((SkillUseTypes)i) && i != SkillFishing) ||
i == SkillAlcoholTolerance || i == SkillBindWound)
continue;

pp->skills[i] = database.GetSkillCap(pp->class_, (SkillUseTypes)i, 1);
}
}
}

But that's assuming you're okay with getting 1359 compilation warnings instead of 1358. :P

[AdrianD]

Superb! Thank you.

I'll have to add something that states something along the lines of: set 1hb = 5 if class = a,b,c,d... and set 1hs = 5 if class = e,f..

I'll have to look at similar to see what the syntax would look like.

Thanks again.

[Shendare]

In the C++ code they have a couple of shortcuts so you don't have to go by individual classes, too.

IsWarriorClass() returns true for WAR, ROG, MNK, PAL, SHD, RNG, BST, BER, BRD.

GetArchectype() returns ARCHETYPE_MELEE, _CASTER, or _HYBRID depending on the class. If you aren't clear on who is and isn't a hybrid, you can see the full list in zone/mob.cpp around line 862.

[AdrianD]

Grateful for that, thank you Shendare.

[Kingly_Krab]

Likely, this would be easier done in Perl because it would be done as soon as the player logged in, rather than being handled by something that may or may not be hard-coded client-side, such as start skills and stuff like that.

[Secrets]

Likely, this would be easier done in Perl because it would be done as soon as the player logged in, rather than being handled by something that may or may not be hard-coded client-side, such as start skills and stuff like that.

Easier, yes, but efficient, no. Perl loops are horrible for performance as it locks the entire zone thread in what's basically a VM to a C++ interface.

[Kingly_Krab]

Easier, yes, but efficient, no. Perl loops are horrible for performance as it locks the entire zone thread in what's basically a VM to a C++ interface.
Yeah, super fun to lock a zone thread by creating a endless loop when using a 'while' loop. But I'm sure this could be simplified in C++ and added to the level up code by creating a client method like Client::MaxSkills or something.

[Shendare]

Or a couple of rules.

Character:MaxSkillsOnCharCreate (true)
Character:MaxSkillsOnLevelUp (false)

[Secrets]

Yeah, super fun to lock a zone thread by creating a endless loop when using a 'while' loop. But I'm sure this could be simplified in C++ and added to the level up code by creating a client method like Client::MaxSkills or something.

No; The loop wouldn't be endless, it would end, but freeze the entire zone while it is processing. This is unacceptable by anyone's standards.

Note clients would still receive data as that's handled in another thread entirely.

The point I was making is it'd be faster to make a method in C++ that you can call from Perl as opposed from looping in Perl.

[AdrianD]

Below are the changes I made, it seems to work but, curious if everything looks right.

Also, I couldn't figure out how to do this using <GetArchectype>. If someone could throw me a bone, I'd appreciate it.

void Client::SetClassStartingSkills(PlayerProfile_Struc t *pp)
{

switch (pp->class_)
{
case WARRIOR:
{
pp->skills[Skill1HSlashing] = 5;
break;
}
case PALADIN:
{
pp->skills[Skill1HSlashing] = 5;
break;
}
case RANGER:
{
pp->skills[Skill1HSlashing] = 5;
break;
}
case SHADOWKNIGHT:
{
pp->skills[Skill1HSlashing] = 5;
break;
}
case BARD:
{
pp->skills[Skill1HSlashing] = 5;
break;
}
case CLERIC:
{
pp->skills[Skill1HBlunt] = 5;
break;
}
case DRUID:
{
pp->skills[Skill1HBlunt] = 5;
break;
}
case MONK:
{
pp->skills[Skill1HBlunt] = 5;
pp->skills[SkillHandtoHand] = 5;
break;
}
case SHAMAN:
{
pp->skills[Skill1HBlunt] = 5;
break;
}
case ROGUE:
{
pp->skills[Skill1HPiercing] = 5;
break;
}
case NECROMANCER:
{
pp->skills[Skill1HPiercing] = 5;
break;
}
case WIZARD:
{
pp->skills[Skill1HPiercing] = 5;
break;
}
case MAGICIAN:
{
pp->skills[Skill1HPiercing] = 5;
break;
}
case ENCHANTER:
{
pp->skills[Skill1HPiercing] = 5;
break;
}
case BERSERKER:
{
pp->skills[Skill2HSlashing] = 5;
break;
}
}
}
Is it necessary to have <break;> or <continue;> at the end?

Thanks

[Shendare]

Do you anticipate that you'll want to make changes on a per-class level later?

If not, you could group them together to clean it up a tad.


void Client::SetClassStartingSkills(PlayerProfile_Struc t *pp)
{
switch (pp->class_)
{
case WARRIOR:
case PALADIN:
case RANGER:
case SHADOWKNIGHT:
case BARD:
pp->skills[Skill1HSlashing] = 5;
break;
case CLERIC:
case DRUID:
case SHAMAN:
pp->skills[Skill1HBlunt] = 5;
break;
case MONK:
pp->skills[Skill1HBlunt] = 5;
pp->skills[SkillHandtoHand] = 5;
break;
case ROGUE:
case NECROMANCER:
case WIZARD:
case MAGICIAN:
case ENCHANTER:
pp->skills[Skill1HPiercing] = 5;
break;
case BERSERKER:
pp->skills[Skill2HSlashing] = 5;
break;
}
}

[Shendare]

Also, you're missing Beastlords. If that wasn't intended, they'd probably be grouped with Monks.

[AdrianD]

BST - oversight, thanks (and clarifying starting skill)

Do you anticipate that you'll want to make changes on a per-class level later?

No, I desire <easy-mode eq: off> and I think, as far as starting skills go, this accomplishes that.

I appreciate your help =)

EDIT: I understand the implications of the corrections, for the most part. I am most thankful for that.

[Uleat]

Yes, Shendare's method is more efficient!


Switch statements essentially create a jump table entry (per case value) and code paths for each entry listed in the machine code.

If no exit clause is specified, the code path will 'fall-through' every line of code until it reaches the default exit clause..meaning that every case between case entry and
default exit will be processed as true.


The fall-through behavior can be very useful for grouping like singular and progressive-compound methodologies..but, it can also be a trap if that behavior is not desired.

[AdrianD]

Thanks Uleat.

I understand the concept but the vocab is still a little beyond me.

I figured to add to this thread with a different snippet since the same question applies. It appears to work with 12 > SkillMeditate > 0 in game.

Both #showstats and watching ticks are accurate although client mana# jumps around. Not sure when it normalizes but, a level 50 at max meditate (235) shows the ticks perfectly for a titanium client.

Does this look right/could it be better?

Comments within the code below:
int32 Client::CalcBaseManaRegen()
{
uint8 clevel = GetLevel();
int32 regen = 0;
if (IsSitting() || (GetHorseId() != 0)) {
if (HasSkill(SkillMeditate) && GetSkill(SkillMeditate) > 11) { // added GetSkill(SkillMeditate) > 11 so obtaining meditate isn't an immediate detriment at low levels
regen = ((GetSkill(SkillMeditate) / 12)) + 1; // changed equation
}
else {
regen = 2;
}
}
else {
regen = 1; // changed to 1 from 2
}
return regen;
}

int32 Client::CalcManaRegen()
{
uint8 clevel = GetLevel();
int32 regen = 0;
//this should be changed so we dont med while camping, etc...
if (IsSitting() || (GetHorseId() != 0)) {
BuffFadeBySitModifier();
if (HasSkill(SkillMeditate) && GetSkill(SkillMeditate) > 11) { // added GetSkill(SkillMeditate) > 11 so obtaining meditate isn't an immediate detriment at low levels
regen = ((GetSkill(SkillMeditate) / 12)) + 1; // changed equation
regen += spellbonuses.ManaRegen + itembonuses.ManaRegen;
CheckIncreaseSkill(SkillMeditate, nullptr, -5);
}
else {
regen = 2 + spellbonuses.ManaRegen + itembonuses.ManaRegen;
}
}
else {
this->medding = false;
regen = 1 + spellbonuses.ManaRegen + itembonuses.ManaRegen; // changed to 1 from 2
}
//AAs
regen += aabonuses.ManaRegen;
return (regen * RuleI(Character, ManaRegenMultiplier) / 100);
}
I'm open to all critique.

Thanks

[image]

I mean is this new formula another guestimation? Since the client has its own idea of regen we should be able to pull the math from there I would think..

[AdrianD]

I have noticed client mana# jumping around on my titanium client for quite some time. I did not take note of it until a few months ago. This occurred at times on live when I played but, probably for different reasons. I never thought much of it.

Prior to making this change, I had changed rule_values regarding mana regen. I noticed mana regen was +2 standing and this was introduced well after classic and possibly after Titanium was relesed, I don't recall. I wanted to fix this so, I changed the rule_values associated with mana regen. This change was not perfect. I knew I would need to alter the source at some point to actually fix it the way I wanted.

As far as guestimating a new formula: This is partially why I posted this. If there is a more accurate formula for mana regen that was in use from classic to OoW or around there, I'd like to hear it.

I played the most on live between SoL and DoN and I do not recall standing mana regen giving +2. In addition, I recall mana regen as a linear function of the meditate skill and never dependant on caster level during the time I played the most. I could be mistaken as I had no reason to actually create a formula.

PS. Your sarcasm is not lost on me ;)

[image]

Wasn't sarcasm honestly, I just think if you want things to work the client and server have to be matching on the functions. I know in the past we had some concerns that our timing of the regen packets and the frequency were possibly a problem.

[AdrianD]

False assumption.

There seems to be quite a few differences between clients. Titanium and others? having certain things hardcoded making customization more difficult.

P99 codes for the same client and I have noticed a little mana# jumping there, as well. Again, I never thought of it much because "on tick" was the real amount, if off by a couple.

This is very noticeable when medding for longer periods and watching HP regen. I'm sure many know what I am talking about.

I'm curious to know how both of these, hp and mana, if different between client/server, could be detrimental.
EDIT: hp and mana regen

Thanks

[image]

Using HP for example while sitting down, it seems the client gets its update and then tries to make another jump just a few second after which then the server corrects back downward. Client is also trying to give 6 hp per tick more than the server wants to in this scenario.

http://i.imgur.com/zVexaQ6.png

[AdrianD]

Aye, this is what I noticed with mana regen but maybe the other way around. The client says +1 because it's titanium, and the server says, nu-uh +2 and adds 1. I was getting 1+1 mana regen on most ticks (always 2 per full tick).

This was the main reason for my fix. I figured to learn a bit and change manaregen to something I thought was more classic-y.

[image]

yea id like to see it fixed also. Plus the scenario in which people fall unconscious before they lose all their HP... if you get hit serious enough you fall over for a moment until for whatever reason it lets you back up as if nothing happened.

[AdrianD]

That stopped happening to me after I recompiled on 7/29/15 and changed some rules. Unsure what it was exactly.

Before that, I would have very brief "double hits".

Check out post #3 - 7 in the thread below. Unsure if it will work for you or if you have done this but, I was very pleased it went away.

http://www.eqemulator.org/forums/showthread.php?t=39907

[Shendare]

FYI, the HP and Mana jumping forward and then back to what it's supposed to be while regenerating? Happened on Live, too. For yeeeeears. Wouldn't be surprised if it still does.

[Uleat]

Character:SoDClientUseSoDHPManaEnd, perchance?

That doesn't solve the entire issue..but, it may have been a rule you changed.


I know that item scaling/stats is also an issue with some clients..but, I think demonstar55 did some work to fix that, or at least alleviate some of the problem.

When the client tries to calculate regen bonuses based on item stats, they really need to be inline with what the client expects..not what the server is necessarily using.

There's still a lot of tweaking that needs to be done in this area.

[AdrianD]

Character:SoDClientUseSoDHPManaEnd, perchance?

That was the last thing I did before it went away, changed the rule to false.

When the client tries to calculate regen bonuses based on item stats, they really need to be inline with what the client expects..not what the server is necessarily using.

I don't foresee it affecting me but, does this mean changing <Character:HPRegenMultiplier> or similar is not the best idea?

[AdrianD]

I'm trying to replace the `CheckIncreaseSkill` equation in `zone\client.cpp( ~2268 )`. After a bit of testing, I found something that will only require changing one line. I don't like the coefficients for some skills. The solution I found mitigates a bit of my distaste for them.

To be replaced:

int32 Chance = 10 + chancemodi + ((252 - skillval) / 20);

I would like some assistance with properly writing the new equation. Below is my attempt to describe it:

I had to graph this as a function of x - f(x) - otherwise I would have done it differently.
Instead of y = x^3 it needed to be x = y^3, I think. It's been many years since I've done any of this.

c = chancemodi
x = skillval

This is directly from the graphing program: (-root(3, 9.61x)1.5 + 22)

The best I can do to write it out: <-root(3, ((|15 + c|^1.1) * 0.4 + 1.75) * x ) * 1.5 + 22 + (2c/3)>

The parts <(-root(3,>, <(|15 + c|^1.1)> I do not know how to write out in C++

This is as far as I got:

int32 Chance = 22 + (chancemodi * 1.5) + 1.5 * (-root (3, (((abs(15 + chancemodi))^1.1 * 0.4 + 1.75) * skillval)))

or

int32 Chance = 22 + (chancemodi * 1.5) + 1.5 * (-cube root of (((abs(15 + chancemodi))^1.1) * 0.4 + 1.75) * skillval))

If you are interested in seeing it graphed with a few comparisons to current skill increase rates:

http://prntscr.com/8kpm36

[AdrianD]

C'mon you programming wizards! With all the traffic today I can't imagine noone can throw me a bone!

Please tell me how to write - (cube root), absolute value and x^n in C++.

[Uleat]

A good place to start: http://www.cplusplus.com/reference/cmath/


cbrt(x)
abs(x)
pow(x, e)

[AdrianD]

Thanks. Knowing how to put them in the lines was my main concern (syntax).

I can reference the functions anytime I forget something, which is often.

Regardless, I got it to work and according to the logsys, it's working as intended, just as the graph shows.

EDIT: btw Uleat, that's the exact page I used to figure it out =)

[AdrianD]

Never said it was pretty:

double param, result;
param = ((pow ((std::abs(15 + chancemodi)), 1.1) * 0.4 + 1.75) * skillval);
result = cbrt (param);
int32 Chance = (-((result)* 1.5) + 22 + (chancemodi / 1.5));


If it could be better, please let me know.

[AdrianD]

I removed the Spellshield and the "cast on other" messages which appear when attacking an npc with a rune. The Spellshield message seemed correct but, the "cast on other" message displays the damage shield "cast on other" each time the npc hits the client. This only occurs if the client has a DS and the NPC has a rune. The "was pierced by thorns" message also appears, as normal.

My main concern is if the second line I commented out will affect anything else. I tested this by casting a rune on myself and having an npc attack and dispel the rune. It appears to work normal. I left larger spaces between lines to accentuate the lines I commented out.


\zone\attack.cpp(3548 - 3571)

//see if any runes want to reduce this damage
if(spell_id == SPELL_UNKNOWN) {
damage = ReduceDamage(damage);
Log.Out(Logs::Detail, Logs::Combat, "Melee Damage reduced to %d", damage);
damage = ReduceAllDamage(damage);
TryTriggerThreshHold(damage, SE_TriggerMeleeThreshold, attacker);

if (skill_used)
CheckNumHitsRemaining(NumHit::IncomingHitSuccess);

} else {
int32 origdmg = damage;


// damage = AffectMagicalDamage(damage, spell_id, iBuffTic, attacker); <--- commented out second


/* if (origdmg != damage && attacker && attacker->IsClient()) { <---
if(attacker->CastToClient()->GetFilter(FilterDamageShields) != FilterHide) <--- commented out first
attacker->Message(15, "The Spellshield absorbed %d of %d points of damage", origdmg - damage, origdmg); <---
} */


if (damage == 0 && attacker && origdmg != damage && IsClient()) {
//Kayen: Probably need to add a filter for this - Not sure if this msg is correct but there should be a message for spell negate/runes.
Message(263, "%s tries to cast on YOU, but YOUR magical skin absorbs the spell.",attacker->GetCleanName());
}
damage = ReduceAllDamage(damage);
TryTriggerThreshHold(damage, SE_TriggerSpellThreshold, attacker);
}


I was trying to find the relationship in this portion with other portions that would tell me why it would make the reference to `cast_on_other` but, couldn't figure it out.

Thanks

[AdrianD]

It's been about a week since I started making changes to my server code. I'm very green in this aspect but, I learn quickly and understand concepts more than syntax. I also understand the implications of things. Not everything though!

I want to thank those that have given assistance if I haven't already.

My question/issue at the bottom.

I've added a few lines to <\zone\zonedb.cpp(3177)> and created a couple rules <\common\ruletypes.h(151) - RULE_CATEGORY(Pets)>.

It works as I expected.

Below is what I added:
ruletypes.h
RULE_BOOL(Pets, PetZonePersistence, true) // if true, pets will zone with players
RULE_BOOL(Pets, PetLogPersistence, true) // if false, pets will poof 30 minutes after logging character out (PetZonePersistence must be true)


zonedb.cpp
void ZoneDatabase::LoadPetInfo(Client *client)
{
// added this part 9/28/15
if (!RuleB(Pets, PetZonePersistence)) {

std::string query = StringFormat("DELETE FROM `character_pet_info` WHERE EXISTS (SELECT * FROM `character_data` WHERE character_pet_info.char_id = character_data.id AND `char_id` = %u AND character_pet_info.pet = 0)", client->CharacterID());
auto results = database.QueryDatabase(query);
if (!results.Success())
return;

query = StringFormat("DELETE FROM `character_pet_buffs` WHERE EXISTS (SELECT * FROM `character_data` WHERE character_pet_buffs.char_id = character_data.id AND `char_id` = %u AND character_pet_buffs.pet = 0)", client->CharacterID());
results = database.QueryDatabase(query);
if (!results.Success())
return;

query = StringFormat("DELETE FROM `character_pet_inventory` WHERE EXISTS (SELECT * FROM `character_data` WHERE character_pet_inventory.char_id = character_data.id AND `char_id` = %u AND character_pet_inventory.pet = 0)", client->CharacterID());
results = database.QueryDatabase(query);
if (!results.Success())
return;
}
else if (!RuleB(Pets, PetLogPersistence)) {

std::string query = StringFormat("DELETE FROM `character_pet_info` WHERE EXISTS (SELECT * FROM `character_data` WHERE character_pet_info.char_id = character_data.id AND `char_id` = %u AND(UNIX_TIMESTAMP(NOW()) - `last_login` > 1800) AND character_pet_info.pet = 0)", client->CharacterID());
auto results = database.QueryDatabase(query);
if (!results.Success())
return;

query = StringFormat("DELETE FROM `character_pet_buffs` WHERE EXISTS (SELECT * FROM `character_data` WHERE character_pet_buffs.char_id = character_data.id AND `char_id` = %u AND (UNIX_TIMESTAMP(NOW()) - `last_login` > 1800) AND character_pet_buffs.pet = 0)", client->CharacterID());
results = database.QueryDatabase(query);
if (!results.Success())
return;

query = StringFormat("DELETE FROM `character_pet_inventory` WHERE EXISTS (SELECT * FROM `character_data` WHERE character_pet_inventory.char_id = character_data.id AND `char_id` = %u AND (UNIX_TIMESTAMP(NOW()) - `last_login` > 1800) AND character_pet_inventory.pet = 0)", client->CharacterID());
results = database.QueryDatabase(query);
if (!results.Success())
return;
}
// above added 9/28/15 and a few small changes below

// Load current pet and suspended pet
PetInfo *petinfo = client->GetPetInfo(0);
PetInfo *suspended = client->GetPetInfo(1);

memset(petinfo, 0, sizeof(PetInfo));
memset(suspended, 0, sizeof(PetInfo));

std::string query = StringFormat("SELECT `pet`, `petname`, `petpower`, `spell_id`, "
"`hp`, `mana`, `size` FROM `character_pet_info` "
"WHERE `char_id` = %u",
client->CharacterID());
auto results = database.QueryDatabase(query);
if (!results.Success()) {
return;
}


My notes during the process:
Added lines to remove pet persistence after 30 minutes when logged off - `ZoneDatabase::LoadPetInfo` - \zone\zonedb.cpp(3177)
- it appears to work but, I am unsure if there is a better way
- this may also delete the entries if zoning after 30 min from `last_login` timestamp (if this timestamp is what I presume)
- it references `last_login` similar to norent although norent items are only loaded once through inventory.cpp, I think
- the solution may be to add the new lines where inventory.cpp is called for norent items
- nevermind, `last_login` appears to be an update timestamp which eliminates this as a possible issue
- create a rule for it to easily switch off for later expansions - done

ToDo: remove pet persistence between zones and create a rule for it - \zone\zonedb.cpp(3177)
- likely need an "if" statement in the same code as above; it seems to call for those lines after each zone switch
- possible solution is to simply use the added lines but, without the timestamp part and add a rule
- added more lines and rules linked to these lines
- it appears to work although I notice the rules need some time to catch up after #rules reload with these added rules
- I'm guessing it has to do with zonedb periodic updates
- issue remains on logging out and keeping pet for 30 min if `PetZonePersistence` is false - currently pets poof when logging back
- possible solution is to reference the DB `character_data` for zoneid prior to overwriting this data when logging in
- unsure the order in which the data is currently read and then overwritten

My implied question:
- issue remains on logging out and keeping pet for 30 min if `PetZonePersistence` is false
- currently pets poof when logging back
- possible solution is to reference the DB `character_data` for zoneid prior to overwriting this data when logging in
- unsure the order in which the data is currently read and then overwritten

The other issue is the running of the above queries and deleting of DB rows each time a player zones. I'd like to make this a little more efficient, if possible. (such as skipping the query if certain values don't exist, which requires another query ofc but, I seek efficiency)

I am open to suggestions.

Thanks

[Uleat]

Check out where this is being called: https://github.com/EQEmu/Server/blob/master/zone/client_process.cpp#L757 ..

.. in this function: https://github.com/EQEmu/Server/blob/master/zone/client_packet.cpp#L1685


Very similar methodology for norent (temporary) items.

[AdrianD]

I pored over NoRentExpired, bool deletenorent and some other areas until I came to zonedb and saw some things I was familiar with. As this is all so foreign to me, knowing what to search for and then processing it consumes much more time than actually changing it to something different.

I was thinking of trying to work it in with the norent code but, my concern was with keeping suspended pets suspended. My intention was to cover as many possibilities as I could think of with that.

The difference with no rent items and what I'm trying to do is the additional option of having pets poof when zoning (a la classic) while not having them go away like norent items when logging for under 30 minutes.

It really isn't a big deal as far as the game goes. I enjoy trying to solve the problem.

Thanks for the response, it got me thinking about it a little different.

[Uleat]

Adding that methodology to LoadPets will cause that to be processed every time that LoadPets is called..as in zone changes.

You may need to split the methodology up to have one portion processed on zone entry, and the other on zone change.


You could create handlers for the actions that you want to process in Database(SharedDatabase?) (ref: https://github.com/EQEmu/Server/commit/a1089fccd6d0402863206a3ea65353f5ed8d8044)

.. and handle the returns appropriately.


I know it's a pain to figure out what does what, where and why..but, it comes with exposure.

[AdrianD]

Yes! That's exactly what I was thinking (in regards to splitting it up) but didn't understand and still having a hard time understanding. I'll probably get it a little with, like you said, more exposure.

[Uleat]

I am extremely slow when it comes to picking up new material...

Intellisense has been my friend since day one!

[AdrianD]

I have to say that feature is wonderful, really helps in learning it.

Compiling the new code now. Hopefully it works.

[rencro]

Epic EMU did some work on pets zoning, can look what they did here:

https://github.com/epicemu/Server/blob/master/zone/zoning.cpp#L329

[AdrianD]

Thank you!

I've used <SetPet(0);> as kinda my mulligan. (I've had a few)

Trying to learn some other ways but...

[AdrianD]

I think I figured the pet rules to work as they should. It took way too long to figure out but, I learned a bit more about the system.

Everything appears to work although client crashed twice during testing. I logged into the emu well over a dozen times.

I am unsure if these rules will trump suspended minion AA. I will need to test that another time.

I put a couple very simple lines into two places and reverted everything I did previous.

near <void Client::BulkSendInventoryItems()> \zone\client_process.cpp(848)

// added 9/30/15 no rent check (this may need fixing when introducing suspended minion)
if (!RuleB(Pets, PetLogPersistence))

SetPet(0);

near <void Client::Handle_OP_ZoneChange(const EQApplicationPacket *app) {> \zone\zoning.cpp(192)

// added 9/30/15 - removes pet on zoning (not on logging)
if (!RuleB(Pets, PetZonePersistence))

SetPet(0);

The first occurs within the <deletenorent> code. I searched this area when I originally tried to do this a day or two ago but, was unsure of code to put in. It was also suggested by Uleat.

The following was put in just before what appears to send the client to the new coords, when zoning. This was a little more tricky because I didn't know how the zoning process worked. I ended up turning on all the logsys features to sleuth it out. Rencro made mention of this although at the time, I couldn't make sense of it.

If this could be improved, please let me know.

Thanks for your help.

[Uleat]

I would probably move your 'log persistence' check to here: https://github.com/EQEmu/Server/blob/master/zone/client_packet.cpp#L1628

.. you could even make the database.LoadPetsInfo(this) call an else clause of the con check (PetLogPersistence == true)

I'm not 100% sure..but, that may clear up your client crashes too. (Looks like the profile packet was already sent to the client - with pet info - and you
were deleting the pet on the server afterwards.)

Mixing system methodologies can lead to issues down the road if you need to make changes, or if changes are made, to a code section.


The 'zone persistence' is probably ok (didn't look at the actual location) .. but, consider what you want to default behavior to be in case of a server crash, client ld, etc...

(Some) players will find a way to manipulate your best thought plans..even accidental actions can lead to exploitable discoveries.

[AdrianD]

Yes, the pets actually show up on the UI for a fraction of a second after logging in. I have been watching what happens in the DB the entire time and...

(Looks like the profile packet was already sent to the client - with pet info - and you
were deleting the pet on the server afterwards.)
...seems to be true, for the log part.

The zone part deletes the entry immediately.

I will make it better. It's not exactly how I want even though it functions.

I'll check this out a little later, let it sit for a bit.

Thanks for your help.

EDIT: I tried as many variations as there are in that exact location you recommended Uleat. Although, I did not try anything after moving the bit to \zoning.cpp. I think I've let it sit long enough.

[AdrianD]

I reverted the bit in \client_process.cpp.

I could use a bit of assistance with something most would consider basic.

I need to insert something that checks the no rent code.

This is as far as I got:
if (RuleB(Pets, PetLogPersistence) == false)
{
SetPet(0);
}

else (RuleB(Pets, PetLogPersistence) == true);
{

database.LoadPetInfo(this);
/*
This was moved before the spawn packets are sent
in hopes that it adds more consistency...
Remake pet
*/
if (m_petinfo.SpellID > 1 && !GetPet() && m_petinfo.SpellID <= SPDAT_RECORDS) {
MakePoweredPet(m_petinfo.SpellID, spells[m_petinfo.SpellID].teleport_zone, m_petinfo.petpower, m_petinfo.Name, m_petinfo.size);
if (GetPet() && GetPet()->IsNPC()) {
NPC *pet = GetPet()->CastToNPC();
pet->SetPetState(m_petinfo.Buffs, m_petinfo.Items);
pet->CalcBonuses();
pet->SetHP(m_petinfo.HP);
pet->SetMana(m_petinfo.Mana);
}
m_petinfo.SpellID = 0;
}
}


This is some of the no rent code:

void Client::RemoveNoRent(bool client_update)


bool deletenorent = database.NoRentExpired(GetName());
if (deletenorent) { //client was offline for more than 30 minutes, delete no rent items
if (RuleB(Inventory, TransformSummonedBags))
DisenchantSummonedBags(false);
RemoveNoRent(false);
}
More than hints please.

Thanks

[Uleat]

I'm not seeing the end goal clearly defined...

You're wanting to check a client's norent items inside of a load pets function?

[Shendare]

Looks like he's wanting pets to poof like No Rent items. By the code snippet above, it looks like the best thing would be to move the "if PetLogPersistance==false { SetPet(0); }" bit to just after the "RemoveNoRent(false);" line within the "if (deletenorent) { }" block.

[AdrianD]

Thanks for the replies.

it looks like the best thing would be to move the "if PetLogPersistance==false { SetPet(0); }" bit to just after the "RemoveNoRent(false);" line within the "if (deletenorent) { }" block.

Ah, I was thinking of going back to that. Thank you.

There are some basic understandings I am missing. Hopefully I'll get it soon and more will open up.

[AdrianD]

Ok.

I added a rule check <\zone\client_packet.cpp(1677)>
if (RuleB(Pets, PetLogPersistence) == true)
{
database.LoadPetInfo(this);
/*
This was moved before the spawn packets are sent
in hopes that it adds more consistency...
Remake pet
*/
if (m_petinfo.SpellID > 1 && !GetPet() && m_petinfo.SpellID <= SPDAT_RECORDS) {
MakePoweredPet(m_petinfo.SpellID, spells[m_petinfo.SpellID].teleport_zone, m_petinfo.petpower, m_petinfo.Name, m_petinfo.size);
if (GetPet() && GetPet()->IsNPC()) {
NPC *pet = GetPet()->CastToNPC();
pet->SetPetState(m_petinfo.Buffs, m_petinfo.Items);
pet->CalcBonuses();
pet->SetHP(m_petinfo.HP);
pet->SetMana(m_petinfo.Mana);
}
m_petinfo.SpellID = 0;
}
}

...and reverted this section <\zone\client_process.cpp(844)>
bool deletenorent = database.NoRentExpired(GetName());
if (deletenorent) { //client was offline for more than 30 minutes, delete no rent items
if (RuleB(Inventory, TransformSummonedBags))
DisenchantSummonedBags(false);
RemoveNoRent(false);
// added 9/30/15 no rent check (this may need fixing when introducing suspended minion)
if (RuleB(Pets, PetLogPersistence) == false)
{
SetPet(0);
}
}


As far as client crash from previous, I am uncertain what was causing it but, the wireless connection it had previous was not good. Getting to login server was difficult. I fixed that and logged a character with a pet (+30 minutes). I did not see the pet window but the pet was still an entry in the DB, like previous.

I will review what you said about this Uleat.

Thanks

EDIT: something isn't right with the norent check - I'll check back but, I've had enough for now

[Uleat]

SetPet() is an inherited function from the class Mob: https://github.com/EQEmu/Server/blob/master/zone/pets.cpp#L542

SetPet(0), when called from the Client class, is essentially doing this:
((Mob*)client_inst)->SetPet(nullptr);

There are no Client class-based methods performed at this level and all that is occurring is that you're setting the memory reference for the client's pet to nullptr.

This is ok for other mob-derived classes that don't save their pet buffs, inventories, etc... (they still clean-up as required, though)

But, for client, you should to handle it somewhere at the Client class level..meaning database clean-up, and deletion of any possible pet instances from memory..otherwise,
you will end up with a memory leak (allocated..but, no longer used memory that is not released back to the system - the losses add up)

(If you handle the action before creating an actual in-memory reference, then you won't need to worry about the clean-up.)

[AdrianD]

I appreciate the explanations. I wouldn't have known if you didn't say anything about it. I will digest this in the coming days and try to apply this if I can.

[AdrianD]

I reverted everything that was working with PetLogPersistence. The issue previous was simple and something I overlooked as I quickly wrote it in as I was out the door.

I went back to \zone\client_process.cpp and added queries to the code.

The results are a little better. The entries are deleted until zoning or casting a new pet when new ones are made. I attempted to add database.SavePetInfo(this); <void ZoneDatabase::SavePetInfo(Client *client)> but the result wasn't desireable. (because of <PetInfo *petinfo = nullptr;> happening first maybe?)

The pet window still appears for a short time and the pet=0 entries are non-existent for the char logged 30+ minutes after initial login. (/q resets the entries - unsure if this = LD)

bool deletenorent = database.NoRentExpired(GetName());
if (deletenorent) { //client was offline for more than 30 minutes, delete no rent items
if (RuleB(Inventory, TransformSummonedBags))
DisenchantSummonedBags(false);
RemoveNoRent(false);

// added 9/30/15 no rent check (this may need fixing when introducing suspended minion - if this was an issue the queries below should fix it)
if (!RuleB(Pets, PetLogPersistence))
{
SetPet(0);

std::string query = StringFormat("DELETE FROM `character_pet_info` WHERE `char_id` = %u AND character_pet_info.pet = 0", CharacterID());
auto results = database.QueryDatabase(query);

query = StringFormat("DELETE FROM `character_pet_buffs` WHERE `char_id` = %u AND character_pet_info.pet = 0", CharacterID());
results = database.QueryDatabase(query);

query = StringFormat("DELETE FROM `character_pet_inventory` WHERE `char_id` = %u AND character_pet_info.pet = 0", CharacterID());
results = database.QueryDatabase(query);
}
}


If I knew how to set these queries up in zonedb.cpp and make the connections to it from client_process, I would have. This really isn't that difficult, the concept, the web of connections. It's the syntax or C++ grammar that is the pain in the ass, the huge hindrance for me. I can figure out how the EQEmu system works just by exposure and asking questions where the answer is fairly simple, as has been mentioned.

Thanks

[Uleat]

I wanted to see if there was an existing method to 'delete' pets from clients..but, the only one I found only deleted the inventories and buffs..so, I'm not sure where that
is handled...

(I mean, when a pet dies, it is deleted, right? Otherwise, it would respawn when a client enters the world.)

[Shendare]

Is there just a Pet->Death()? lol

[Uleat]

Well, I followed case GETLOST: back, and SavePetInfo() .. nothing I saw removed the actual pet instance.

I dunno? :P


I know I'm missing something...

[AdrianD]

There is a depop in \spawn2.h and \spawn2.cpp:

void Depop();

void Spawn2::Depop() {
timer.Disable();
Log.Out(Logs::Detail, Logs::Spawns, "Spawn2 %d: Spawn reset, repop disabled", spawn2_id);
npcthis = nullptr;
}

I saw this while sifting through potential options.

There is different depop for zone.

[AdrianD]

A bit of progress, not much though.

No entry being created like it does when zoning and using <SetPet(0);>

\zonedb.cpp - last 1/2 commented out, unsure what to do to make it create a new blank entry

When the part /* */ is uncommented it only removes `spell_id` in the DB - pet_struct maybe?
void ZoneDatabase::DeletePetInfo(Client *client)
{
std::string query = StringFormat("DELETE FROM `character_pet_info` WHERE `char_id` = %u AND `pet` = 0", client->CharacterID());
auto results = database.QueryDatabase(query);
if (!results.Success())
return;

query = StringFormat("DELETE FROM `character_pet_buffs` WHERE `char_id` = %u AND `pet` = 0", client->CharacterID());
results = database.QueryDatabase(query);
if (!results.Success())
return;

query = StringFormat("DELETE FROM `character_pet_inventory` WHERE `char_id` = %u AND `pet` = 0", client->CharacterID());
results = database.QueryDatabase(query);
if (!results.Success())
return;

// PetInfo *petinfo = nullptr;
/*
for (int pet = 0; pet < 2; pet++) {
PetInfo *petinfo = client->GetPetInfo(pet);
if (!petinfo)
continue;

query = StringFormat("INSERT INTO `character_pet_info` "
"(`char_id`, `pet`, `petname`, `petpower`, `spell_id`, `hp`, `mana`, `size`) "
"VALUES (%u, %u, '%s', %i, %u, %u, %u, %f) "
"ON DUPLICATE KEY UPDATE `petname` = '%s', `petpower` = %i, `spell_id` = %u, "
"`hp` = %u, `mana` = %u, `size` = %f",
client->CharacterID(), pet, petinfo->Name, petinfo->petpower, petinfo->SpellID,
petinfo->HP, petinfo->Mana, petinfo->size, // and now the ON DUPLICATE ENTRIES
petinfo->Name, petinfo->petpower, petinfo->SpellID, petinfo->HP, petinfo->Mana, petinfo->size);
results = database.QueryDatabase(query);
if (!results.Success())
return;
query.clear();
} */
}

\client_process.cpp
bool deletenorent = database.NoRentExpired(GetName());
if (deletenorent) { //client was offline for more than 30 minutes, delete no rent items
if (RuleB(Inventory, TransformSummonedBags))
DisenchantSummonedBags(false);
RemoveNoRent(false);

// added 9/30/15 no rent check (this may need fixing when introducing suspended minion - if this was an issue the queries below should fix it)
if (!RuleB(Pets, PetLogPersistence))
{
SetPet(0);

database.DeletePetInfo(this);
}
}


Thanks

[AdrianD]

Added to \client_process.cpp - it's a little smoother, adds the blank row and saves it. The client still shows the pet window momentarily. I would prefer to put <memset(&m_petinfo, 0, sizeof(struct PetInfo));> in the DeletePetInfo part and then run w/e queries are needed to fill in the blanks allowing the removal of <database.SavePetInfo(this);> below.

I would prefer this whole process done a little earlier. Just a guess but, somewhere near <case ServerOP_SyncWorldTime:> (\zone\worldserver.cpp(744).

This also leaves out what Uleat said before about memory loss? which I have no clue about.

\client_process.cpp(841)
// added 9/30/15 no rent check (this may need fixing when introducing suspended minion - if this was an issue the queries below should fix it)
if (!RuleB(Pets, PetLogPersistence))
{
SetPet(0);

database.DeletePetInfo(this);

memset(&m_petinfo, 0, sizeof(struct PetInfo));

database.SavePetInfo(this);
}
}

[AdrianD]

Last one for a bit:

The two lines below SetPet, <Mob* mypet =......etc.>, gives the same resulting appearance as <SetPet(0);>. (takes a moment to depop) I am unsure if one is better than the other.

\client_process.cpp(841)
if (!RuleB(Pets, PetLogPersistence))
{
SetPet(0);

// or

Mob* mypet = this->GetPet();
mypet->CastToNPC()->Depop();

database.DeletePetInfo(this);

memset(&m_petinfo, 0, sizeof(struct PetInfo));

database.SavePetInfo(this);
}
}

[AdrianD]

A little more progress on the pet thing.

The entire delete/save process (creating a new blank entry in table `character_pet_info`) for the DB in one spot. This eliminates running a couple unneeded queries from using the <SavePetInfo> function.

The pet window still appears on client. I don't know if this is avoidable by doing the norent check somewhere earlier in the process. It's not a big deal unless there is a cleaner/more efficient way to do this.

\client_process.cpp
bool deletenorent = database.NoRentExpired(GetName());
if (deletenorent) { //client was offline for more than 30 minutes, delete no rent items
if (RuleB(Inventory, TransformSummonedBags))
DisenchantSummonedBags(false);
RemoveNoRent(false);

// added 9/30/15 no rent check (updated 10/3/15)
if (!RuleB(Pets, PetLogPersistence))
{
SetPet(0);

database.DeletePetInfo(this);
}
}

\zonedb.cpp
void ZoneDatabase::DeletePetInfo(Client *client)
{
std::string query = StringFormat("DELETE FROM `character_pet_info` WHERE `char_id` = %u AND `pet` = 0", client->CharacterID());
auto results = database.QueryDatabase(query);
if (!results.Success())
return;

query = StringFormat("DELETE FROM `character_pet_buffs` WHERE `char_id` = %u AND `pet` = 0", client->CharacterID());
results = database.QueryDatabase(query);
if (!results.Success())
return;

query = StringFormat("DELETE FROM `character_pet_inventory` WHERE `char_id` = %u AND `pet` = 0", client->CharacterID());
results = database.QueryDatabase(query);
if (!results.Success())
return;

PetInfo *petinfo = client->GetPetInfo(0);

memset(petinfo, 0, sizeof(struct PetInfo));

int pet = 0;

query = StringFormat("INSERT INTO `character_pet_info` "
"(`char_id`, `pet`, `petname`, `petpower`, `spell_id`, `hp`, `mana`, `size`) "
"VALUES (%u, %u, '%s', %i, %u, %u, %u, %f) "
"ON DUPLICATE KEY UPDATE `petname` = '%s', `petpower` = %i, `spell_id` = %u, "
"`hp` = %u, `mana` = %u, `size` = %f",
client->CharacterID(), pet, petinfo->Name, petinfo->petpower, petinfo->SpellID,
petinfo->HP, petinfo->Mana, petinfo->size, // and now the ON DUPLICATE ENTRIES
petinfo->Name, petinfo->petpower, petinfo->SpellID, petinfo->HP, petinfo->Mana, petinfo->size);
results = database.QueryDatabase(query);
if (!results.Success())
return;
query.clear();
}

As an aside: While going through this process I noticed some queries are ran anywhere between 2-6 times from this maybe? <database.QueryDatabase(query);>

I understand my tinkering may have caused some of this but, I'm curious to know the purpose of this.

Thanks

EDIT:

Not sure if I mentioned this before. I had to add the line below when I created the item in \zonedb.cpp. Can't take it for granted.

\zonedb.h
void DeletePetInfo(Client *c);

[AdrianD]

Good day.

I created a rule to force training of certain melee skills. The intention is to only force training if the skill is acquired after level 1.

For Riposte below,

\zone\attack.cpp(412)
if (IsClient()) {
if (!RuleB(Skills, TrainMeleeSkills)) {
CastToClient()->CheckIncreaseSkill(SkillRiposte, other, -10);
}
else if (RuleB(Skills, TrainMeleeSkills) && CastToClient()->HasSkill(SkillRiposte)) {
(CastToClient()->CheckIncreaseSkill(SkillRiposte, other, -10));
}
}

I'm concerned about what will happen when the rule is true and the character does not have <SkillRiposte>. Do I need some kind of escape from this if it goes beyond <else if>? There is quite a bit more code after what is quoted above.

This question applies to several similar issues I had concern about.

Thanks

[Uleat]

If there's nothing left to process in the function after your 'else if' and the exclusion can be caught with an 'else' or another 'else if' statement, you can simply handle it with a 'return' statement.

Otherwise, you might consider putting your post-con check code inside of a single con check that meets all of the requirements for that code to be processed.

[AdrianD]

Thank you sir.

If there's nothing left to process in the function after your 'else if' and the exclusion can be caught with an 'else' or another 'else if' statement, you can simply handle it with a 'return' statement.

I understand this.

Otherwise, you might consider putting your post-con check code inside of a single con check that meets all of the requirements for that code to be processed.

I'm a little fuzzy on this.

I can't find it but I thought I saw an instance where <else> was used with nothing between { } except spaces.

EDIT: Here but this may not apply to my issue.
else {
if (!GetFeigned() && (DistanceSquared(bindmob->GetPosition(), m_Position) <= 400)) {
// send bindmob bind done
if(!bindmob->IsAIControlled() && bindmob != this ) {

}
else if(bindmob->IsAIControlled() && bindmob != this ) {
// Tell IPC to resume??
}
else {
// Binding self
}
// Send client bind done

bind_out->type = 1; // Done
QueuePacket(outapp);
bind_out->type = 0;
CheckIncreaseSkill(SkillBindWound, nullptr, 5);

int maxHPBonus = spellbonuses.MaxBindWound + itembonuses.MaxBindWound + aabonuses.MaxBindWound;

[Uleat]

Just wasted code space..though it may be there, with a remark, to indicate what is happening to arrive there or an area for future implementations.

The compiler will probably optimize out that particular else clause since there is nothing being processed.

[AdrianD]

Gotcha, thanks.

[AdrianD]

I've tried a different approach with no success, so I try here:

I'm trying to come up with a script which, I hope, will summon a corpse using the necro spell, id = 3 (summon corpse).

I've looked at the Dragons of Norrath and the guild lobby quests and they will no do what I want them to do. I want to summon a corpse with the same behavior as the NEC spell "summon corpse".

Everything appears to work except self targeting. Since the npc is targeted at the end of the script and the spell is self (player) cast, the text stating "Your target must be a group member for this spell." appears. I've also tried to change the actual spell effect code.

Below is what I have so far with <location of target code> to indicate where I think it should belong.

-- global\a_dungeon_necromancer.lua NPCID

function event_say(e)
if(e.message:findi("hail")) then
e.self:Say("I know the reason you come to see me, " .. e.other:GetName() .. ". Don't be afraid, the living do not concern me... You require my [services], you must ask for them. Only the willing are allowed when living.");
elseif(e.message:findi("services")) then
e.self:Say("" .. e.other:GetName() .. ", to perform the necessary incantation, you must hand me one platinum, three gold, three silver and seven copper pieces.");
end
end

function event_trade(e)
local item_lib = require("items");
local leet_cash = 0;
if(item_lib.check_turn_in(e.trade, {platinum = 1, gold = 3, silver = 3, copper = 7})) then

leet_cash = 1;
end

if(leet_cash >= 1) then

-- <location of target code>

eq.SelfCast(3);
if(leet_cash == 1) then
e.self:Say("Believe me when I say this exact amount represents the services offered.");
leet_cash = 0;
end

end
item_lib.return_items(e.self, e.other, e.trade)
end


If anyone knows how to accomplish what I am trying to do, please share.

Thanks

[provocating]

I do not even know why I am trying to help you at this point, but whatever.

You never explained WHY the guild summoner scripts could not be modified to do what you want. I mean they summon all of the players corpses. How does that not work?

[AdrianD]

I do not even know why I am trying to help you at this point, but whatever.

You never explained WHY the guild summoner scripts could not be modified to do what you want. I mean they summon all of the players corpses. How does that not work?

I'm sorry if I hurt you, provocating. I can tell you for certain, I do not feel the same way. Grudges aren't healthy.

If I knew how to alter <EVENT_SUMMON();> to only work in specific zones or with specific corpses, I would not be asking the question.

Maybe it's clarification I need. I don't have much knowledge of scripts and this is why I asked the question to begin with, hoping, someone could clarify this.

[AdrianD]

$client->SetTarget($client);

That's all that was needed in the .pl file.

# global\a_dungeon_necromancer.pl NPCID


sub EVENT_SAY {
if ($text=~/hail/i) {
quest::say("I know the reason you come to see me, $name. Don't be afraid, the living do not concern me... You require my [services], you must ask for them. Only the willing are allowed when living.");
}
if ($text=~/services/i) {
quest::say("$name, to perform the necessary incantation, you must hand me one platinum, three gold, three silver and seven copper pieces.");

}
}

sub EVENT_ITEM {
if (plugin::takeCoin(1000)) {
quest::say("Believe me when I say this exact amount represents the services offered and becomes transmuted into the much larger pile of where, I only know. HAHA!");
$client->SetTarget($client);
quest::selfcast(3);
}
plugin::returnUnusedItems();
}

Thanks

PS. Could use a writer.

[Noport]

Feel free to try this have no idea where npc location or for what zone i had no way to test this perl script.

a_dungeon_necromancer.pl

EVENT_SAY {
if ($text=~/hail/i) {
quest::say("I know the reason you come to see me, $name. Don't be afraid, the living do not concern me... You require my [services], you must ask for them. Only the willing are allowed when living.");
}
if ($text=~/services/i) {
quest::say("$name, to perform the necessary incantation, you must hand me one platinum, three gold, three silver and seven copper pieces.");
}
}
sub EVENT_ITEM{
if (($platinum>=1) (($Gold>=3) ((Silver>=3) (($Copper>=7{)) then
$npc->SetAppearance(0);
$client->Message("Believe me when I say this exact amount represents the services offered.");
quest::summonburriedplayercorpse($charid, $x, $y, $z, 0);
$corpse = 0;
$charid = 0;
else{
quest::say("Thank you for your donation $name, it wasn't enough though ...");
}
}
sub EVENT_ITEM {
quest::say("I have no use for this, $name.");
plugin::return_items(\%itemcount);
}

[AdrianD]

Thank you Noport.

[AdrianD]

I added a mechanic to <channelchance> and am wondering if it looks right or if it could be done better. It may be a little confusing but the purpose was learning as much as adding this code. It's an exercise in futility trying to channel at lower levels and the added mechanic mitigates this a bit. It also reinforces those who practice skills when they level, like the old days.

Also, the <distance_moved> and <distancemod> formulas do not do as advertised. The idea seems proper but the implementation is off. I will add to this my results when I get to it.

The big thing I am wondering is if the code in the header is returning the value for the previous level max skill. This mechanic seems to work upon initial testing.

\zone\client.h(695)
inline uint16 PrevMaxSkill(SkillUseTypes skillid) const { return MaxSkill(skillid, GetClass(), GetLevel()-1); }

\zone\spells.cpp(1105) - <void Mob::CastedSpellFinished>
// max 93% chance at 252 skill
channelchance = 30 + GetSkill(SkillChanneling) / 400.0f * 100;

uint16 cS = CastToClient()->GetRawSkill(SkillChanneling); // current skill
uint16 mS = CastToClient()->MaxSkill(SkillChanneling); // current level max skill
uint16 pS = CastToClient()->PrevMaxSkill(SkillChanneling); // current level - 1 max skill
double iA = 20 / (mS - pS); // adjustment interval
double cA = 0; // cA = adjustement coefficient
double param, result;
param = (pow((.0005*cS), 2));

while (true)
{
if (cS >= 200) {
break;
}
else if (cS <= pS) {
break;
}
else if (GetLevel () <= 1) {
cA = (cS * 2);
result = cA - param;
channelchance += result;
break;
}
else {
cA = ((cS - pS)*iA);
result = cA - param;
channelchance += result;
}
break;
}
channelchance -= attacked_count * 2;
channelchance += channelchance * channelbonuses / 100.0f;
}

<snip code>
distance_moved = d_x * d_x + d_y * d_y;
// if you moved 1 unit, that's 25% off your chance to regain.
// if you moved 2, you lose 100% off your chance
distancemod = distance_moved * 25;
channelchance -= distancemod;
}

Thanks

EDIT: here is a visualization of the behavior http://prnt.sc/8s311y

The blue line is the current, unadjusted channelchance. The lower red function is the formula I added. The upper red function is max skill channelchance after adding the the formula.

[AdrianD]

Also, the <distance_moved> and <distancemod> formulas do not do as advertised. The idea seems proper but the implementation is off. I will add to this my results when I get to it.


I think I misinterpreted this. Disregard.