puzzlepaint / freeage

An open-source reimplementation of the game engine of Age of Empires 2: Definitive Edition.
58 stars 4 forks source link

Damage types, armor and damage calculation. #15

Closed MaanooAk closed 4 years ago

MaanooAk commented 4 years ago

Melee and Pierce damage are just a DamageType, no special handling.

(I had implemented this once before...)

I don't know how are you planning to do unit/building type modifications from tech and civ bonuses, but this implementation could work both like immutable and/or mutable.

Waiting on feedback :)

MaanooAk commented 4 years ago

@puzzlepaint If you have something in mind that is needed but you don't want to implement, I'm open to suggestion...

MaanooAk commented 4 years ago

The windows failed failed was originating here:

    armor.SetValue(DamageType::Melee, 0);
    CHECK_EQ(armor.GetValue(DamageType::Melee), 0); // fail

I start looking for a bug in the code, but the fact that the linux build was running without a fail could be 2 things:

I couldn't find a "undefined behaviour", so I tried doing stupid stuff to see what happens. One idea was that the array is inited in a none 0 value, so some optimazation could thing that setting to 0 could be ignored. So I first added this and worked:

    armor.SetValue(DamageType::Melee, 1);
    CHECK_EQ(armor.GetValue(DamageType::Melee), 1); // ok
    armor.SetValue(DamageType::Melee, 0);
    CHECK_EQ(armor.GetValue(DamageType::Melee), 0); // ok

Then I tried to play it smart with:

    armor.SetValue(DamageType::Melee, 1);
    armor.SetValue(DamageType::Melee, 0);
    CHECK_EQ(armor.GetValue(DamageType::Melee), 0); // fail

And then I went to this (which also looks like code that could be written without this "bug" in mind):

    CHECK_EQ(armor.GetValue(DamageType::Melee), Armor::None); // ok
    armor.SetValue(DamageType::Melee, 0);
    CHECK_EQ(armor.GetValue(DamageType::Melee), 0); // ok

I suggest, push this now, and I will try revisit it when I will have access to a windows machine (because now I was making a change and waited for 10 minutes to see the result form the ci). I'm also thinking about posting a StackOverflow question. Any thoughts?

EDIT: The insane thing is that there is only an array, no C++ crazy thing.

puzzlepaint commented 4 years ago

Thanks for the update! Some general ideas:

If it's not a memory corruption thing, then my impression is that it could be worth trying to reduce it to a minimal working example and report as a possible compiler bug.

puzzlepaint commented 4 years ago

As expected, valgrind does not report any error.