MdsParser design proposal

Try commented 5 years ago

Hi, I have an idea how to make Mds parser better.

Problem

In current design we have ModelScriptBinParser/ModelScriptTextParser with base class ModelScriptParser. Every time someone want's to add support for new chunk-type, code has to be written twice: once for Bin, once for Txt.

Possible solution

Move more logic into ModelScriptParser base class. It can be done, by introducing a new group of virtual functions, like:

      virtual std::string readStr()=0;
      virtual std::string readKeyword()=0;
      virtual uint32_t    readDWord()=0;
      virtual int32_t     readI32()=0;
      virtual float       readFloat()=0;
      ...

Prototype

Here is a prototype, I have in my fork: https://github.com/Try/ZenLib/blob/master/zenload/modelScriptParser.cpp Testing: run locally Gothic2Notr, Gothic2, Gothic1, Gothic reloaded. Test suite: https://github.com/Try/ZenLib/blob/master/tests/test_mds.cpp (sorry, but no test-data in repo - I've had to use dumps from game assets)

Benefits

Less code
Feature-consistency over gothic1-2
Write once - debug everywhere king of code

Cons

Debug everywhere king of code, because one line can change behavior for both formats.
CHUNK_MESH_AND_TREE doesn't match between txt and bin formats - implemented as virtual function.
Prototype contains unrelated changes(more chunks + removal of get-function in favor of public access)
CHUNK_ERROR handling not implemented in prototype - text parser is always relaxed.

Andre, welcome back by the way :)

ataulien commented 5 years ago

I haven't thought of that, but it seems like a great idea!

I'll look into it more closely tomorrow when I have more than my phone.

ataulien commented 5 years ago

Sorry for taking so long. The new design looks really nice! You even managed to half the length of the source file!

I'm not so worried about the cons you listed though, the benefits outweigh them by a lot IMHO.

Try commented 5 years ago

OK) But now we need to make a plan: how to merge this change in original ZenLib. The trouble:

my fork diverted very much from original
change is very large for merge: blind cherry-pick - can break things by accident

So, what we can do: 1) I can clone regoth+vanilla version of ZenLib to prepare proper pull-request( gonna take some time) 2) You can cherry-pick commit and do alignment, by yourself

ataulien / ZenLib