TrinityCore / WowPacketParser

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

Change UpdateFields properties to be nullable #641

Closed BAndysc closed 2 years ago

BAndysc commented 2 years ago

Changed UpdateField types so that their properties with simple types are now nullable, to better reflect what they actually store: when the value is not send by the server, the field will be null.

Also, update handlers no longer modify existing global data, instead they return an object with updated fields and those are later merged with the global object storage.

Use with: https://github.com/Shauren/wow-tools/pull/8

Tested with: 4.3.4, 6.2.3, 7.2.5, 7.3.5, 8.1.5, 8.2.0, 8.2.5, 9.0.1 both txt and sql output, all are equal

MaxtorCoder commented 2 years ago

System.Nullable??????? Excuse me.

mdX7 commented 2 years ago

System.Nullable??????? Excuse me.

Theres a more nice way to actually suggest stuff @MaxtorCoder. Theres even the option to explain why something should be used or not..

MaxtorCoder commented 2 years ago

System.Nullable??????? Excuse me.

Theres a more nice way to actually suggest stuff @MaxtorCoder. Theres even the option to explain why something should be used or not..

He used the correct way before xddd

Shauren commented 2 years ago

Autogenerated code, if you want int? instead of System.Nullable<int> then go submit a PR to microsoft to change what System.CodeDom.Compiler.CodeDomProvider.GetTypeOutput returns

BAndysc commented 2 years ago

Autogenerated code, if you want int? instead of System.Nullable<int> then go submit a PR to microsoft to change what System.CodeDom.Compiler.CodeDomProvider.GetTypeOutput returns

Exactly, thanks for the comment 😅


Overall, is this approach fine? Can I proceed with adjusting old handlers to nullable?

Shauren commented 2 years ago

Yeah this is fine

BAndysc commented 2 years ago

So from my side it is ready to be merged

MaxtorCoder commented 2 years ago

Looks good, and my apologies for my earlier comments 😄👍🏼