raziel23x / skyrim-plugin-decoding-project

Automatically exported from code.google.com/p/skyrim-plugin-decoding-project
1 stars 3 forks source link

Requesting code review on that one because I with to merge the refactoring in the main branch now ;) #151

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What needs to be reviewed?
All difference between this and the trunk

Describe how it is now.

Describe the desired change.

Outline the goals/benefits of the enhancement.

Please provide any additional information below.
thanks

Original issue reported on code.google.com by HuguesLe...@gmail.com on 16 Nov 2013 at 4:09

GoogleCodeExporter commented 9 years ago
What exactly (exp, sharlikran) to review and where?
There were so much changes in those branches, I lost count long time ago :)

Original comment by zila...@gmail.com on 16 Nov 2013 at 5:30

GoogleCodeExporter commented 9 years ago
from exp to trunk.

They should be "identical" in functionality, well except for CanHandleAlso 
now...

Original comment by HuguesLe...@gmail.com on 16 Nov 2013 at 5:34

GoogleCodeExporter commented 9 years ago
All I can see:
1) refactoring
2) more checks and debugging messages
3) some changes to ByteArray prefixes handling, don't know what exactly they 
are for
4) TES5Saves stuff which I honestly consider unimportant unless it is 
functional and has a purpose. Already removed unfinished and useless D3D and 
nif code before from xEdit. I'm afraid TES5Saves code might become the same 
some time later, considering Fallout4 is coming next year (we'll know in 3 
weeks) and development will shift entirely to it.
Idk it's current state. Does it work and what exactly it allows atm?

Original comment by zila...@gmail.com on 16 Nov 2013 at 6:13

GoogleCodeExporter commented 9 years ago
On 4, as far as my test goes.
TES5save from sharlikran branch is functional (in TES5) for all the form types 
decoded, and  for dumping the VMAD to its individual components, ie down to 
xEdit data type.

In exp, I included all "general" definitions used by Saves as a way to verifiy 
they do not have negative effect on current functionning.

Once that is done and validated, I would start merging the rest of TES5saves in 
the main branch, and make it "public".

Sorry if I was was'nt clear on my goals before.

Original comment by HuguesLe...@gmail.com on 16 Nov 2013 at 6:25

GoogleCodeExporter commented 9 years ago
Is it possible to move all "saves" code to wbInteraceSaves and 
wbImplementationSaves? Not sure about bloating common modules with a game 
specific code which is not even for plugins.
I know this will probably require refactoring wbInterface since a lot of stuff 
is under implementation.
But in the end do whatever you think must be done. The majority on changes were 
done by you for almost a year, you know far better than me :)

Original comment by zila...@gmail.com on 16 Nov 2013 at 6:36

GoogleCodeExporter commented 9 years ago
Normally it's already done :) but a second look can always help.
What remains are either shared interface definition of modifications to base 
types, like referencing private members or non public classes.

By the way, on your earlier 3) Most of those changes implemented byte arrays 
with a specified length which I encountered while decoding the saves. Well 
until I realized, such byte arrays, were in fact character strings :(
The only remaining use of that is the wbNull definition record that helps 
properly display empty union branches.

Original comment by HuguesLe...@gmail.com on 16 Nov 2013 at 6:52

GoogleCodeExporter commented 9 years ago
I'm for any changes unless they break something :)

Original comment by zila...@gmail.com on 16 Nov 2013 at 7:06

GoogleCodeExporter commented 9 years ago
Closing. Merging to begin soon :)

Original comment by HuguesLe...@gmail.com on 11 Dec 2013 at 9:16