Better game / entity code

Issue #113 resolved
Carsten Fuchs
created an issue

The current game / entity code is cumbersome in many ways.

It should be improved as outlined at

At the same time, it may or may not a good opportunity to revise the relationship of the related entity classes: - server vs. client entity classes, - engine vs. game entity classes, - concrete derived entity classes.

Comments (24)

  1. Carsten Fuchs reporter

    (In [528]) Net code: Added two new methods:

    void BaseEntityT::Serialize(NetDataT& Stream) const;
    void BaseEntityT::Deserialize(NetDataT& Stream);

    These two methods are currently nowhere used, and both their signature as well as their implementation is temporary only. However, they are an important conceptual cornerstone for implementing #113, and subsequent changes related to #113 will use them before they are eventually brought into their final shape.

    References #113.

  2. Carsten Fuchs reporter

    (In [529]) Net code: Replaced the "remove me!" bit of `SC1_EntityUpdate` messages with a separate new message type `SC1_EntityRemove`. This eliminates the multi-purpose nature of the data in `SC1_EntityUpdate` messages.

    References #113.

  3. Carsten Fuchs reporter

    (In [530]) Net code: Fix direct public access to `EntityStateT` instances.

    `EntityStateT`s are private members of entities that we strive to entirely remove in the course of #113 as described at References to them used to be passed around publicly in user code, violating the encapsulation of entity internals and effectively preventing further refactoring of the code. The changes in this revision improve the class interfaces to enhance encapsulation and to make further refactoring possible.

    References #113.

  4. Carsten Fuchs reporter

    (In [531]) Net code: Changed the interpolation code for NPC entities to work without direct access to `EntityStateT`. (The current interpolation "solution" is a hack anyway that should be thoroughly redone.)

    References #113.

  5. Carsten Fuchs reporter

    (In [532]) This change updates all user code to no longer directly access the public `EntityStateT State` member of `BaseEntityT` instances, except for those accesses that are going to be subsumed by `BaseEntityT::Serialize()` and `BaseEntityT`::Deserialize()` later.

    References #113.

  6. Carsten Fuchs reporter

    (In [533]) Net code: Introduce new classes for managing the game entity state.

    The `BaseEntityT` methods

    void BaseEntityT::Serialize(cf::Network::OutStreamT& Stream) const;
    void BaseEntityT::Deserialize(cf::Network::InStreamT& Stream, bool IsIniting);

    previously introduced in r528, have been updated accordingly.

    References #113.

  7. Carsten Fuchs reporter

    (In [536]) Net code: - Added a new Wireshark dissector for the Cafu Engine network protocol. - Removed the obsolete and long unmaintained "ReadDump" tool.

    The "ReadDump" tool was a weak attempt to accomplish what has always been better done in Wireshark, especially now that we have a new Cafu-specific protocol dissector for it. The wireshark-cafu.lua dissector, although not yet entirely complete, is already very useful and subject to further improvement.

    References #113.

  8. Carsten Fuchs reporter

    (In [537]) Net code: Properly deal with `SC1_EntityRemove` messages: Cannot remove an entity from the delta frame that wasn't in the delta frame in the first place.

    This was a *very* old and long outstanding bug that fortunately never manifested itself over the years. It occurs however when a complete `SC1_FrameInfo` message gets too large, so that the server, as an emergency measure, drops some of the related `SC1_EntityUpdate` sub-messages. When the client then receives the *next* `SC1_FrameInfo` message, a related `SC1_EntityRemove` sub-message may refer to an entity that is not in the clients corresponding delta frame, possibly applying all subsequent delta updates to the wrong entity.

    `SC1_FrameInfo` message normally don't get large enough to trigger this, but in the first step of our implementation of #113 we use uncompressed messages that easily start the above chain of events.

    This change fixes the problem.

    References #113.

  9. Carsten Fuchs reporter

    (In [538]) This revision implements the first fundamental change for #113: Use the new methods `BaseEntityT::Serialize()` and `BaseEntityT::Deserialize()` to serialize/deserialize the entity state to/from `cf::Network::StateT` instances. With the entities doing this work themselves, the network code no longer implements and is abstracted from entity state serialization/deserialization: It now deals with the resulting `cf::Network::StateT` instances that from its "users" perspective are just opaque data containers. The Cafu network protocol has been updated accordingly.


    This revision is self-contained, but it is '''NOT''' good for public use!

    It addresses the basic state handling on the client and server, but it is "incomplete" in the sense that: - the prediction code has not yet been updated accordingly, - there is no (RLE-)compression for the generated delta messages.

    These points (to be addressed in separate upcoming commits), plus inherent limitations regarding the maximum message length in the Cafu network protocol, make it seem like this revision was utterly broken.

    In order to make this revision "work", - `usePrediction = false` is required at the in-game console, - number `1250` must be changed to `500` (!) in line 278 in `EntityManager.cpp` (then recompiled), - there will still be entities (appear to be) missing if you walk through the maps.

    References #113.

  10. Carsten Fuchs reporter

    (In [539]) A direct continuation of r538, this change updates (and fixes) the client prediction code to work with the new, opaque `cf::Network::StateT` entity state representation as well.

    References #113.

  11. Carsten Fuchs reporter

    (In [540]) This change completes the work begun in r538, implementing RLE-compression for the delta messages.

    There is a test built in that compares our RLE-compression implementation with the DEFLATE compression algorithm. When activated, the written `compress_log.txt` file typically looks like this:

    227 bytes in original delta message
    33 bytes in deflate-compressed message, compression result is 0 (Z_OK)
    28 bytes in RLE-compressed message.
    227 bytes in original delta message
    12 bytes in deflate-compressed message, compression result is 0 (Z_OK)
    6 bytes in RLE-compressed message.
    227 bytes in original delta message
    12 bytes in deflate-compressed message, compression result is 0 (Z_OK)
    6 bytes in RLE-compressed message.
    227 bytes in original delta message
    32 bytes in deflate-compressed message, compression result is 0 (Z_OK)
    28 bytes in RLE-compressed message.
    227 bytes in original delta message
    34 bytes in deflate-compressed message, compression result is 0 (Z_OK)
    27 bytes in RLE-compressed message.

    In our (limited) tests, not a single case occurred where the RLE-compressed message was larger than the deflate-compressed one.

    This change, together with r539, remedies the caveats mentioned in r538: Cafu should be good for public use again (even though there is more work to be done for #113).

    References #113.

  12. Carsten Fuchs reporter

    (In [543]) Move the entity event handling and processing from the `EngineEntityT` into the `BaseEntityT` class, where it belongs.

    This change eliminates the last direct "external" accesses to the `StateT` member of the `BaseEntityT` class.

    References #113.

  13. Carsten Fuchs reporter

    (In [545]) This change prepares moving some core data members of class `EntityStateT` into class `BaseEntityT`. It does so by updating the constructor signatures, so that it looks to user initialization code as if the data members had already been moved.

    This preliminary step is supposed to make the subsequent actual changes easier to review.

    References #113.

  14. Carsten Fuchs reporter

    (In [546]) As prepared in r545, this change moves some core data members of class `EntityStateT` into class `BaseEntityT`.

    It's now possible to dissolve the `EntityStateT` class entirely, and/or to move it into the concrete derived entity classes.

    References #113.

  15. Carsten Fuchs reporter

    (In [547]) Splitted the `BaseEntityT::Serialize()` and `BaseEntityT::Deserialize()` methods to call the new `BaseEntityT::DoSerialize()` and `BaseEntityT::DoDeserialize()` methods, respectively.

    This follows the "Non-Virtual Interface Idiom" as described by Scott Meyers in "Effective C++, 3rd Edition", item 35 ("Consider alternatives to virtual functions.").

    Also removed the old method `Cl_UnserializeFrom()`, refactoring it into implementations of `DoDeserialize()`.

    References #113.

  16. Log in to comment