Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SnapEntity class #43

Open
wants to merge 10 commits into
base: target-gd3.2
Choose a base branch
from

Conversation

ExplodingImplosion
Copy link

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?

entityinfo.gd:
create_instance now works properly

assertions for get_properties_from_node for debugging

snapshot.gd:

get_entity now returns blank array instead of null

snapshotdata.gd:

instead of checking for if entities are null (artifact from when snapshot entities were objects), checks if they're empty or not

decode_delta now caches entity, cmask and id in the function (also fixed issue where tried to access key/member from array instead of index)

replaced instances of clone_entity with entity.duplicate

netmain.gd: set up in accordance with refactored code

unit.gd:

added apply_state func so it gets parsed properly

clutter_base.gd: set up in accordance with refactored code

glowprojectile.gd: set up in accordance with refactored code

p3dchar_base.gd: set up in accordance with refactored code
fixed default property value not working

turns out theres some new bugs instead :) fuck
clutter_base.gd: desperately tried to make stuff work how its supposed to

p3dchar_base.gd: desperately tried to get net vars to work by separating actual net vars stuff from what gets modified (literally changes nothing to my knowledge)

glowprojectile.gd: stopped quantizing net orientation to avoid some weird ass bugs
entityinfo.gd: fixed replication by shifting the compare indexes

replicating clutter is still LMFAOOOOOOO (everything wonks the fuck out)
clutter_base.gd: clutter base now syncs properly and doesnt freak out

p3dchar_base.gd: got rid of redundant variables

megamain.gd: idle time now expressed in ms
unit.gd:

added net_position and net_orientation vars

added functionality so that global_transform properly updates them at the end of frames and properly assigns itself to them at the beginning of frames

got rid of some commented out depreciated code

snapshotdata.gd:

cleaned away some commented code I used for debugging

clutter_base.gd:

cleaned some depreciated commented away code

added a comment to explain a weird code pattern

modified some previous comments to make more sense

glowprojectile.gd:

got rid of depreciated commented code and rewrote some old comments for more accuracy

p3dchar_base.gd:

now significantly more congruent with the main branch's logic, comments revamped, old commented code removed

KNOWN ISSUES:

server player doesn't replicate to clients
clutter desync's
connected player doesn't correct itself (was broken last commit too i think?)
@Kehom
Copy link
Owner

Kehom commented Nov 9, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants