Combat Fixes

[James76]

OK, the attack.cpp file replacement is ready. Just subtle changes throughout. The major change is damage bonus.... That monks get +20%, and at level 50 (and every five levels) you gain +15% damage. It looked like that's what was intended in the original source, but the math was screwing it up.

Here is it.

- Test the ripostes if you like, the discipline is Furious for warriors, at level 56.

- Combat damage significantly increased.. both for you and against you.. Lets see if its still balanced? Solo-raids should become more difficult.

I still have my database mobs ATK and Accuracy set to my last stated update based on level. I'd like people to test that and let me know if the code's defaults should be more on par with that, or find better generic values.. Maybe raid mobs should get 2x the values? You get the idea.

Enjoy the ripostes! They can trigger now on every attack instead of every hit.

[James76]

Anybody bring this in to their builds?

[renoofturks1]

I am hoping to test it tonight.

[James76]

These combat fixes were so important to the emulator, but they appear to be overlooked? There should also be an additional change, a riposte should beat an "automatic hit", since I recall a duel I had between my warrior and a ranger, he triggered auto hits, and I wielded the slowest 2H with the highest damage I found at that time, activated auto-riposte, and he died in seconds. So, is this still getting looked at? Is anybody using this? Seems like changes are like trying to move mountains. I'm just trying to shake things up and make everything better.

[Andrew80k]

I totally missed this. I will add it to my server tonight and start playing with it.

[trevius]

James, I think part of the problem might be that you changed that file so much and have very limited comments in it that doing a diff against the current version of the file shows almost nothing matches up. This makes it nearly impossible for anyone to have any idea of what is going on other than reading your code line by line and figuring out what it does the hard way.

Also, there have been many combat changes starting around the time that you released this file. Many of them have been looked at very closely and everyone had input into the resulting final code. The recent changes to the damage bonus code are quite amazing. And there have been quite a few others. With the complete reformat of the file from it's original state, it would make adding these new changes a very involved process.

It would be best if you could simply post the changes you made individually the way that KLS asked for in the code submissions sticky like this:

In attack.cpp, change this:

somecode

to this:

somenewcode

or

In attack.cpp, remove this line:

somelineofcode

and replace it with this:

somenewlineofcode

Then we can see exactly what you are changing and easily evaluate if your changes are a good improvement.

I realize that it is hard to post it in that format when there are many changes, but it shouldn't take nearly as long as it took to write the actual code.

The main problem here is that everyone on the project has limited time to invest in it. We do have many new people with the ability to update the code that were added recently, so we can definitely get changes moving much quicker than ever. But I think asking anyone to completely evaluate the entire attack.cpp to many compare and find what has been changed is asking alot and would require quite a bit of time.

Not that it actually means anything, but I see that your join date is Sep 2008. I almost have to assume that you have been either involved or following this project for quite some time even though your account is new. This is because for someone to join for the first time and within a few days to rewrite one of the core components of the entire EQEmu code, is nearly impossible. If you have been involved in the project before, it wouldn't hurt to know who you were. Due to the amount of time it would take to evaluate your changes in the code, having some kind of reputation would certainly escalate the chance of one of the capable people to actually check it out. Currently, the only people I would trust completely to rewrite attack.cpp would be KLS and Derision.

Replacing an entire file makes it hard to track down and resolve the cause of problems/bugs if any are found. Putting your file in and testing it out is definitely an option, but I don't see any way this could be put into the official source download until the entire thing was evaluated. Which is why I think if it is possible, you should try to post your changes to it line by line. The problem with us trying to diff your file against the official one is that you reformatted the entire file moving brackets around, changing tabs into spaces, and other reformatting. I don't think cleaning up a file format is bad, but it isn't something you really want to do at the same time you are doing other changes. I would try to get the changes done first, then maybe take the official file and not change anything in it other than the format and resubmit that. By doing both at the same time, it just makes reading the diff a pretty involved process instead of just seeing clearly what code was changed.

I am sorry if any of this offended you in any way, as it wasn't meant to. I hope you can understand my perception of why your submission hasn't had very much attention. Some of the other mods and devs may have different reasons, but this is just my take on it. With all of the normal code submissions that have been made lately and some that each of the devs/mods have been working on themselves, it is hard to find the time needed to devote to evaluating your file changes. But, I am positive that if you posted them the normal way, they would definitely get reviewed and if they were found to be an improvement of fix, they would get added ASAP.

Most importantly, I hope you aren't upset by me saying any of this. I certainly don't speak for the entire team. And we definitely don't mean to scare off talented, helpful and productive coders. Please be patient with us, and if possible, try to keep code submissions in the standard format so we can quickly and easily review the code and then test it. But, always feel free to post if you have any concerns or if you think your code is being ignored or may have been forgotten about. We really appreciate any and all code submissions as does the rest of the community when they get to reap the benefits of it

[KLS]

Yeah basically Trev hit it. If I diff the file against the current attack.cpp and every line comes back, which it does in this case, it's basically impossible to review.

With the riposte thing, it's not so much that we're not taking it into account it's that avoid and check hit is backwards and no one has taken the time to fix it. So I think you have the right idea but maybe the wrong approach from what I saw.

If we could get something where we could actually see what changes are made it would certainly help.

[trevius]

I would even say to maybe try posting a couple of your major changes first in 1 post and then maybe start a new thread for all of the minor stuff. That should keep it from being too overwhelming and the major stuff would get quick attention.

[James76]

Alrighty, I've made two lists of changes, one for the combat fixes, and one for the generic type warnings and general use of code. The file link I posted above included all of them, I don't believe I missed anything although I may have, and you will notice them when you compile. That is, you shouldn't get any warnings anymore when you execute 'make' for the zone executable, if all other ".o" files were already made except attack.o. To anyone who doesn't already know, after you "make zone", delete attack.o, and run make again -- that is what I'm talking about. My posted file link also included a uniform formatting scheme, making the file 500% more readable. Some of the code in the original file had skewed blocks which made tracing slightly difficult. One of the companies I worked for had issued to us a uniform coding methodology document which I've been applying ever since, which made all projects readable by everyone so there wasn't ever any confusion. You will also notice that the new variables I create include a type-hint in its name, which was also part of that referred document.

To address Trevius' statement about being a relatively new member, I had been following this project for some time, though I haven't put my mind towards applying changes. It wasn't until I basically quit all MMO's and I remembered this old project, and I came back to see how far it came, and started up my own server for development. I have been coding C++ specifically for about 20 years, and I have a knack for code that allows me to just jump in on any project, because its clearer to me than reading english. This project only uses perhaps 20-30% of the potential of C++, according to Strustrup's own definitive language guide. It can get 1000% more confusing when you see all that is possible, and even then its readable to me with no issues at all. I have always had this gift, if you would like to call it that, ever since I started coding when I was 6 years old on an IBM XT in BASIC. I knew how to code before learning how to read, I don't know how or why, its just always been natural. Maybe I learned how to read by using those "learn to program BASIC" manuals with those type-them-in starter programs. But for certain, I understood math functions like ABS(), exponents, and BEDMAS when they were teaching the other kids Addition. When I was 7 I traced the source code of a learning game called Lemonade Stand, to remove the ABS() from an input line when you buy your components, during a class break on a Commodore PET, so that you could buy & sell like it was a free market. My teacher was most definitely NOT impressed when I showed I could make millions, because she saw it like an act of destroying school property, even though the change was not permanent in any way.... But I digress.

All these changes refer to an unedited attack.cpp from build 1129. I'll give the line number (or range) with a colon, and the required change. Sometimes I'll include an unchanged first line of a multi-line modification.

You may want to work from the bottom-up, so as not to become confused by line-number skewing, and preferably from the very bottom of this entire post.

Ok, the riposte fix, and some combat math fixes are as follows:

342:
  if (damage >= 0 && CanThisClassRiposte() && !other->BehindMob(this, other->GetX(), other->GetY()))


956-969:
  int nBonus = 0;
  int nLevel = GetLevel();

  if(GetClass() == MONK)
    nBonus += 20;
  if(nLevel > 50)
    nBonus += 15 * ((nLevel - 45)/5);
    
  float fBonus = 1.0f + static_cast<float>(nBonus)/100.0f;
  min_hit = static_cast<int>(static_cast<float>(min_hit) * fBonus);
  max_hit = static_cast<int>(static_cast<float>(max_hit) * fBonus);


978:
    min_hit = static_cast<int>(static_cast<float>(min_hit) * static_cast<float>(100 + itembonuses.MinDamageModifier + spellbonuses.MinDamageModifier) / 100.0f);


995:
  other->AddToHateList(this, 0);
  other->AvoidDamage(this, damage);


1526:
  damage = 0;  //miss
  other->AvoidDamage(this, damage);

OK, now for the type warning fixes, as well as the "++" or "--" modifications. These type fixes are not some sort of psychotic application of static_cast, it was only applied to replace the older "C" type-cast ing of (int) with static_cast(int), or because the math led me to believe (with careful analysis) a cast may or should be required. You will notice I did not type-cast everything, which would be a mark of insanity, because the compiler -will- apply casts when risk of loss of information is nil, without warning. For example, an int to a float is no risk, and no casting required.

LINE 174 & 175:
  chancetohit += static_cast<int>(145.0f * static_cast<float>(level_difference) / 100.0f);
  chancetohit -= static_cast<int>(static_cast<float>(defender->GetAGI()) * 0.015f);

347:
  bonus += 2.0f + static_cast<float>(skill)/35.0f + static_cast<float>(GetDEX())/200.0f;

364:
  bonus = 2.0f + static_cast<float>(skill)/35.0f + static_cast<float>(GetDEX())/200.0f;

383:
  bonus = static_cast<float>(defender->spellbonuses.ParryChance + defender->itembonuses.ParryChance) / 100.0f;

385:
  bonus += 2.0f + static_cast<float>(skill)/35.0 + static_cast<float>(GetDEX())/200.0f;

405:
  bonus = static_cast<float>(defender->spellbonuses.DodgeChance + defender->itembonuses.DodgeChance) / 100.0f;

407:
  bonus += 2.0f + static_cast<float>(skill)/35.0f + static_cast<float>(GetAGI())/200.0f;

477:
  uint32 intervalsAllowed = 20;

517:
  acfail = static_cast<int>(atof(tmp) * 100.0f);

532:
  acrandom = static_cast<int>((atof(tmp)+1.0f) * 100.0f);

537:
  damage -= static_cast<int>(GetAC() * acreduction/100.0f);

633:
  if(weapon_item->BaneDmgBody == static_cast<uint32>(against->GetBodyType())){

650:
  if(weapon_item->BaneDmgBody == static_cast<uint32>(against->GetBodyType())){

706:
  for(int x = 0; x < 5; ++x){

738:
  for(int x = 0; x < 5; ++x){

767:
  dmg += static_cast<int>(eledmg * against->ResistSpell(weapon_item->GetItem()->ElemDmgType, 0, this) / 100);

771:
  for(int x = 0; x < 5; ++x){

775:
  dmg += static_cast<int>(weapon_item->GetAugment(x)->GetItem()->ElemDmgAmt * against->ResistSpell(weapon_item->GetAugment(x)->GetItem()->ElemDmgType, 0, this) / 100);

783:
  if(weapon_item->GetItem()->BaneDmgBody == static_cast<uint32>(against->GetBodyType())){

801:
  for(int x = 0; x < 5; ++x){

803:
  if(weapon_item->GetAugment(x)->GetItem()->BaneDmgBody == static_cast<uint32>(against->GetBodyType())){

822:
  if(weapon_item->GetItem()->BaneDmgBody == static_cast<uint32>(against->GetBodyType())){

840:
  for(int x = 0; x < 5; ++x){

922:
  //int8 otherlevel = other->GetLevel() ? other->GetLevel() : 1; //unused

932:
  weapon_damage = static_cast<int>(static_cast<float>(weapon_damage) * (100.0f+static_cast<float>(bonus) / 100.0f));

1473:
  if(weapon->BaneDmgBody == static_cast<uint32>(other->GetBodyType())){

1482:
  eleBane += static_cast<int16>(weapon->ElemDmgAmt * other->ResistSpell(weapon->ElemDmgType, 0, this) / 100);

1724:
  for (uint32 i = 0; i < MAX_GROUP_MEMBERS; ++i){

1766:
  for(int i=0;i<6;++i) { // Doesnt work right, needs work

2099:
  for (int i = 0; i < MAX_PROCS; ++i)

2495-2498:
  ProcBonus += static_cast<float>(itembonuses.ProcChance + spellbonuses.ProcChance) / 1000.0f;
  ProcChance = 0.05f + static_cast<float>(mydex) / 9000.0f;
  ProcBonus += (ProcChance * static_cast<float>(AABonus)) / 100.0f;

2532:
  for(r = 0; r < MAX_AUGMENT_SLOTS; ++r) {

2542:
  ProcChance *= (100.0f+static_cast<float>(aug->ProcRate))/100.0f;

2568:
  float WPC = ProcChance*(100.0f+static_cast<float>(weapon->ProcRate))/100.0f;

2594:
  //float procmod =  static_cast<float>(GetDEX()) / 100.0f + ProcBonus*100.0f;  //did somebody think about this??? //unused

2607:
  int chance = static_cast<int>(ProcChance + static_cast<float>(SpellProcs[i].chance));

2831:
  for(int bs = 0; bs < BUFF_COUNT; ++bs){

2838:
  --buffs[bs].numhits;

Please don't read into any of my statements as though I'm being snide or crass, I'm not using any emotion, and I did not have any hurt feelings by your remarks. I understand making change can be difficult without proper source control in a fully trusted environment. A project such as this would not exist if it were up to one person alone, so this is expected. But please accept all of my changes, I've given every line in the original attack.cpp great thought.

[James76]

I just noticed I use variable nLevel once, when it was intended to replace all further uses of GetLevel(), before I created the math statement to apply bonus damage for increments of 5 levels at 50+. Before it was calling GetLevel() 4 or 5 times. It was too late for me to edit the post, so maybe include that little tweak when implementing. That is, replace all further uses (only one more exists in that method) of GetLevel() with nLevel.

[trevius]

I am glad you weren't upset with my post

I do think that making the code format uniform and cleaning up any compile warnings is awesome. But, if/when you ever decide to do it again, please only do it without making other changes so we know what to expect and what to look for. Making code changes and format changes at the same time makes it too hard to tell what is happening. Probably waiting until the next official release to reformat any files would be a good idea, so we don't have to try to fit the new code changes since the last update in retro-actively. The quicker we can get a new reformat in after a new release, the better.

And on your note about the GetLevel adjustment, are you saying that this code:

  int nBonus = 0;
  int nLevel = GetLevel();

  if(GetClass() == MONK)
    nBonus += 20;
  if(GetLevel() > 50)
    nBonus += 15 * ((nLevel - 45)/5);

Should actually be changed to this:

  int nBonus = 0;
  int nLevel = GetLevel();

  if(GetClass() == MONK)
    nBonus += 20;
  if(nLevel > 50)
    nBonus += 15 * ((nLevel - 45)/5);

and no other changes? Or am I misunderstanding what you are saying?

I can edit that change into your post if I know exactly what you wanted set.

Then we can officially test out the code and get it added

[James76]

Originally Posted by trevius

and no other changes?

Yep, you got it.

[trevius]

Tried to add some of this code to my source tonight and am unable to tell if I need to add in parts of code, replace part, or remove parts.

If you can post something like:

After this:

SomeCode

Add this:

SomeMoreCode

or

Remove this:

SomeCode

And replace it with this:

SomeNewCode

That would make it ALOT easier to get this code added and tested. Sorry about all of the extra work to break it down, but it is still hard to add it in without knowing exactly what is needing to be removed, added, or replaced.

[James76]

Well, I figured if I posted 80 or 100 blocks of one-liner code fields, it would look much more confusing than some simple one-line changes, ... All it takes is to look at the source code, find the line, and change it.

The "riposte and combat math" fixes part, let me eludicate......

342: CHANGE THE LINE

956-969: CHANGE LINES 956-969 WITH THE FOLLOWING

978: CHANGE THE LINE

995: YOU WILL SEE AddToHateList(), insert AvoidDamage() below.

1526: YOU WILL SEE damage=0, insert AvoidDamage() below.

The rest of the changes are simply changes.. I've been holding back any further work until this gets integrated, if this is such a problem maybe I should just wait for CVS access and merge changes myself, if CaveDude EVER gets back to me... This is starting to turn into some kind of joke.

[James76]

Actually, this is clearly becoming too difficult, some simple changes that all it takes is to look at the source code and apply some logic. I could hand this to any of my co-workers, and they have never looked at the source code before, and they wouldn't have any problems I can guarantee you. If they did then they shouldn't have a job as a programmer. So I'm just going to step out of this, no hard feelings, I'm just tired of people passing themselves off as programmers when they aren't, and then I have to justify any little change when I could pass the list of changes to somebody like my project manager and he'd just nod at each line understandingly. An actual team of developers would be flying through changes and code, and instead I feel like any little thing that gets done is always "working against the grain" so-to-speak. I could continue on this path but my foresight is telling me I'm just going to have a lot more of this than necessary, and its just starting to be a complete waste of my time.