search

Kehom / GodotAddonPack

A collection of pure GDScript addons for Godot
http://kehomsforge.com
MIT License
190 stars 15 forks source link

Remove SnapEntity class #43

Open ExplodingImplosion opened 1 year ago

ExplodingImplosion commented 1 year ago

Why get rid of SnapEntity?

When working with the networking addon, I grew increasingly frustrated when working with the SnapEntity class. It felt like there was too much overhead with this idea of having 2 separate sets of variables, plus re-implementing a "data getting", "data setting" function for every new SnapEntity, and passing around a dictionary to change variables on another script... For most of my use cases that included simple "grab every changed network variable" and "set every changed network variable", I figured it would be easier for me to understand by cutting out the middle man, so to speak. So I started a version of the GodotAddonPack that entirely removed the SnapEntity class.

How is this implemented?

Previously, the network singleton grabbed every registered SnapEntity class and put them in the _entity_info section of snapshot data or something. I'm a bit fuzzy on the details, as I'll explain later. This got changed to checking EVERY script in the project and instead of grabbing all their variables, under the circumstance that they have any variables prefixed with "net_", it registers the script in the _entity_info dictionary... executing similar steps, but with some changed behavior. Honestly it's just better to take a look at the differences between the previous and new entityinfo.gd scripts.

Here's the big deal: SnapEntitys are now Arrays. Because these changes "unifies" the networking behavior of entities, (and because the SnapEntity script is literally gone from the project files), EntityInfo, snapshots, etc pass around Arrays.

Now every time a script receives a network snapshot, there's a guarantee that the update from the snapshot will modify every "net variable" that was sent in the snapshot. Furthermore, there is a guarantee that every time a script creates its own entity in a snapshot, it'll construct a properly ordered Array containing copies of all its net variables.

Ok, but...

There are certainly advantages to using SnapEntity. And these advantages are part of why I've questioned even making this public in the first place. This feels more like a "nice to have for specific users" feature. A great demonstration of the aforementioned advantages is that multiple behavioral scripts can use the same SnapEntity script to handle their side of network serialization. This is (almost) showcased in the networking demo, where each type of player character uses the same SnapEntity. In theory each of these character types could have their own functionality, without needing to modify or create a new SnapEntity class. It also allows scripts to have greater control over which variables are modified during the networking step of a frame, versus simulation. Being able to easily decide which networked variables impact node variables that change its behavior is pretty great! You can still do this with the addon by, for instance, creating separate variables within the script itself. If you want to change an object's velocity using a networked velocity variable, simply call it net_velocity. But if you want to only change it in accordance with network updates based on your own logic, you can create a net_velocity variable and a separate velocity variable. Another mild disadvantage of the current implementation is that "redundant" scripts (that have all the same networked variables but are tied to different functionality) won't get grouped together in the snapshot data's entity info dictionary.

Why release this a month after I last worked on it?

I want to be clear-- THIS CODE IS BROKEN. lmao. I haven't worked on it for about a month after university work picked up, and splitting my time between this, prototyping my game, and working on replays, I was already spread thin. Also, when initially working on this change, I took a very gung-ho, "just delete SnapEntity from the addon pack and fix everything as you go" approach. Currently, like almost everything is broken. I forget specifics, besides projectiles being really weird and clutter barrels totally desyncing across clients.

At this point, I've forgotten some of the implementation details, I've forgotten some of the bugs, and as with Replays, I've forgotten what I was last specifically working on in the project. Truly a gamer moment out of me. The thing is, I'm still interested in finishing these projects, it's just that I've been busy. And that's not gonna change any time soon. It's just much easier for me to talk about this project than to actually work on it. This PR is just here to spark discussion. What do you guys think?

Kehom commented 1 year ago

I understand very much that the SnapEntity system can be cumbersome. Yet, it's the way I have found to not impose certain restrictions. It's extremely versatile in the sense that you don't have to enforce certain naming conventions nor require a specific node hierarchy.

Now, the major problem here is that it breaks compatibility, which is something that have prevented me from improving several of the addons in this pack.