search

TrinityCore / WowPacketParser

World of Warcraft Packet Parser
GNU General Public License v3.0
410 stars 352 forks source link

Fix parsing of post 52038 classic wotlk SMSG_ON_MONSTER_MOVE #812

Closed killerwife closed 5 months ago

killerwife commented 7 months ago

@funjoker related to your recent pushes. Breaks builds which are > 52038 on wotlk classic or vanilla

funjoker commented 7 months ago

why does it break wotlk classic? and why does it break vanilla?

Vanilla does not fallback to wotlk_classic, does it?

funjoker commented 7 months ago

i totally hate the fallback mechanics nowadays ._.

killerwife commented 7 months ago

The old check is based on build number. Builds fall back depending on what they were side by side developed with. Destination is kept atm on 3.4.3, but on retail branch its gone. It will likely be changed in next major wotlk classic version. You have to judge it case by case. Have yet to check if it breaks season of discovery client, but thats TBD

funjoker commented 7 months ago

still don't get the point here.

For Wotlk_Classic: 3.4.0 -> 2.5.1 -> 9.0.1 -> 1.13.2 -> 8.0.1 -> 7.0.3 -> 6.0.2

For TBC Classic: 2.5.1 -> 9.0.1 -> 1.13.2 -> 8.0.1 -> 7.0.3 -> 6.0.2

For Vanilla Classic: 1.13.2 -> 8.0.1 -> 7.0.3 -> 6.0.2

Which changes did break the stuff in your opinion? 72cba6a942e3fac5886a985ba5f2eca33b6be0ae this only added a 3.4.3 only handler to 3.4.0 handler, so it only affects 3.4.3

funjoker commented 7 months ago

oh it's not about my changes. It's about people messing around with handlers.

Imo SL and upper stuff does not belong to 8.0.1 handler. I'd like to see versioned code inside own handlers.

this 10.2. change belongs into V10 module imo, even if it means renundant code. Otherwise we can break A LOT of parsing for classic stuff yes. I donno who started messing with mixing modules

killerwife commented 7 months ago

3.4.3 is spun on top of 10.7 afaik, 3.4.2 and 3.4.1 is 10.2, 3.4.0 is 10, the list goes on. Blizz each version synchs most opcodes to retail, the major outlier being SMSG_UPDATE_OBJECT which I always fork. Hence it makes perfect sense if you want minimal changes. If we copypaste the handler to the classic lib as well, its gonna be exact the same 99% of the time from what I have looked at.

funjoker commented 7 months ago

ye but atm the checks will break everything for classic if placed into lower modules. Will discuss that

killerwife commented 7 months ago

I think the idea was to have less copypaste work since most of the time these hardly change.