gtker / wow_messages

Auto generated messages for the World of Warcraft network protocol
Apache License 2.0
25 stars 10 forks source link

AuraUpdate NOT_CASTER check #97

Open lucas-jones opened 1 month ago

lucas-jones commented 1 month ago

I think this should be a NOT check?

WoW Messages

if (flags & NOT_CASTER) {
    PackedGuid caster;
}

AzerothCore

if (!(flags & AFLAG_CASTER))
    data << aura->GetCasterGUID().WriteAsPacked();

Links: https://github.com/gtker/wow_messages/blob/db051bf33ac723320e95db96e7b2c4e29ad928c6/wow_message_parser/wowm/world/spell/smsg_aura_update_all.wowm#L23

https://github.com/azerothcore/azerothcore-wotlk/blob/0c271cf832b8a6334d38bef7d173e13ee041f8e2/src/server/game/Spells/Auras/SpellAuras.cpp#L271

lucas-jones commented 1 month ago

Though I'm not sure the syntax to solve this...

Maybe we could set the NOT_CASTER value to 0xF7 (bitwise negation of 0x8 for now)

gtker commented 1 month ago

I think setting NOT_CASTER to the negated value is the best option.

Otherwise the solution would be to have a giant chain of statements like

if (flags & EFFECT_1) {
    PackedGuid caster1;
} else if (flags & EFFECT_2) {
    PackedGuid caster2;
} etc.

but that's also a little annoying for codegen.

lucas-jones commented 1 month ago

Ahh not too sure the 0xF7 solution works

gtker commented 1 month ago

Does it work if you make it the OR of all the other flags?

gtker commented 1 month ago

Ah wait, I'm stupid. If we make it 0xF7 then it requires all other enumerators to be active at the same time in order to be valid.

A big chain might be the best solution afterall.

lucas-jones commented 1 month ago

0xF7 was my silly idea 😆

Might need to implement the NOT bitwise operator ~

Also it also needs to check that spell != 0 too

Something like:

struct AuraUpdate {
    u8 visual_slot;
    Spell spell;
    AuraFlag flags;
    Level level;
    u8 aura_stack_count;

    if (spell != 0) {
        if (flags ~ NOT_CASTER) {
            PackedGuid caster;
        }
        if (flags & DURATION) {
            u32 duration;
            u32 time_left;
    }
    }
} {
     versions = "3.3.5";
}

AuraApplication::BuildUpdatePacket

gtker commented 1 month ago

Think it also needs to check that spell != 0 too

I just saw that as well. :)

I don't support using anything other than enumerators in if statements right now since it complicates codegen for downstream users.

I'm not sure what the best solution is just yet, but mostly for things that require weird things that would be difficult to implement I end up just implementing them manually (like UpdateMask, AuraMask, PackedGuid).

The same with (flags ~ NOT_CASTER). I know it's not pretty having a giant chain of else ifs, but right now I don't intend on implementing support for a ~ operator since it's only used on this one occasion and the more weird one-off operators there are the more documentation I need to write and consider how they interact with other weird one-off features.

Could you for now just fix this by doing the else if chaining and then we'll keep the issue open to get a more complete fix at a later point?

lucas-jones commented 1 month ago

Yeahh, It's kind of a shame as I now have to implement readAuraUpdate which requires a reference to a generated flag - AuraFlag. I was hoping to keep generated code isolated

gtker commented 1 month ago

Which language are you generating for?

You'll need to mix generated and non-generated at some point anyway since things like AchievementDoneArray require using a generated struct.

lucas-jones commented 1 month ago

Which language are you generating for?

Javascript/Typescript

You'll need to mix generated and non-generated at some point anyway since things like AchievementDoneArray require using a generated struct.

Ah I see, alright then! I'll accept that I have to reference generated code 😄

Thanks for the help

gtker commented 1 month ago

Javascript/Typescript

Cool, do you intend on making it public at some point?

I'll gladly link to it in the docs so people don't have to spend time figuring out how to generate stuff if there's already a library for it.

Ah I see, alright then! I'll accept that I have to reference generated code

I also had this problem when making the Python version. I ended up just writing the definition manually into the same autogenerated file. For C# and Rust I used the better module system to have it in a separate directory, but still present it to the user as if it had been generated.

For now I think making it

struct AuraUpdate {
    u8 visual_slot;
    Spell spell;
    optional spell_not_null {
        AuraFlag flags;
        Level level;
        u8 aura_stack_count;

        if (flags & EFFECT_1) {
            PackedGuid caster1;
        } else if (flags & EFFECT_2) {
            PackedGuid caster2;
        } else if (flags & EFFECT_3) {
            PackedGuid caster3;
        } else if (flags & SET) {
            PackedGuid caster4;
        } else if (flags & CANCELLABLE) {
            PackedGuid caster5;
        } else if (flags & DURATION) {
            PackedGuid caster6;
        } else if (flags & HIDE) {
            PackedGuid caster7;
        } else if (flags & NEGATIVE) {
            PackedGuid caster8;
        }
        if (flags & DURATION) {
            u32 duration;
            u32 time_left;
        }
    }
} {
     versions = "3.3.5";
}

Would solve your problem within the current limitations of the language.

Depending on what you generate this can become annoying though.