TrinityCore / WowPacketParser

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

Allow to create packet structures for a specific build version. #849

Closed RioMcBoo closed 4 months ago

RioMcBoo commented 4 months ago

I've read discussions here about how Blizzard is making the principles of introducing new packages and their structure more and more confusing. The project is becoming more and more confusing. Therefore, I propose to add small changes so that you can add the desired structure for the desired version of the client without worrying about breaking the cascade system.

How it works? 1) You need to use the BuildMatchAttribute indicating the specific version of the client build for the structure you are interested in. 2) It is advisable that this structure be implemented in a project that is closest to your version of the client. 3) If there is a conflict with older opcodes, use a new overload for ParserAttribute with a boolean value 4) You do not have to create a new structure, but only mark an existing one with BuildMatchAttribute (the existing structure must be implemented in the project that most closely matches the client version or the project that will be added using the cascade system).

For example: https://github.com/TrinityCore/WowPacketParser/pull/850

Shauren commented 4 months ago

Useless, already supported - just add a min/max build on Parser attribute

public ParserAttribute(Opcode opcode, ClientVersionBuild addedInVersion, ClientVersionBuild removedInVersion) is the constructor you are looking for

RioMcBoo commented 4 months ago

Useless, already supported - just add a min/max build on Parser attribute

public ParserAttribute(Opcode opcode, ClientVersionBuild addedInVersion, ClientVersionBuild removedInVersion) is the constructor you are looking for

You don't even understand what the point is. Your restrictions work in such a way that you need to enter the number of the build in which the package is no longer current. However, what if the database does not contain the following assembly? - Do I need to create a crutch like “BuildVersion +1?

We go further - if we use an overload with only one boundary, we cannot quickly find in the code the very structure that processes the opcode in this immediate case. By entering a specific mark in which particular build it works, it is immediately visible and easy to find through the search.

In any case, at a minimum, changes must be made to the current system so that it allows this to be done

RioMcBoo commented 4 months ago

And, as far as I understand, your system assumes that there can only be one implementation (if they were loaded) of the same opcode in one solution. Therefore, if I simply add a new implementation and introduce the necessary restrictions, I will end up with a conflict. In order not to create a conflict, I will have to do something with the old implementation and limit its maximum build. However, some project that references the current project and uses its implementation through a cascade system may suffer because of this.

That's why I made this decision.

RioMcBoo commented 4 months ago

I just want to say that it is becoming noticeable and clear that changes in package structures are no longer happening as linearly as before. Conflicts constantly arise in the cascade system; it becomes too confusing. I looked at this through the eyes of a beginner, and I want to note that this is an extremely confusing system and discourages everyone who wants to contribute to the project.

My solution not only covers the problem with conflicts in the cascade system - you can simply implement the opcodes that you need for your version, without delving into existing ones and without disrupting the operation of an already established cascade system.