-
Notifications
You must be signed in to change notification settings - Fork 15
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
base: target-gd3.2
Are you sure you want to change the base?
Remove SnapEntity class #43
Conversation
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?)
I understand very much that the 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. |
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 newSnapEntity
, 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 theSnapEntity
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:
SnapEntity
s are now Arrays. Because these changes "unifies" the networking behavior of entities, (and because theSnapEntity
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 sameSnapEntity
script to handle their side of network serialization. This is (almost) showcased in the networking demo, where each type of player character uses the sameSnapEntity
. In theory each of these character types could have their own functionality, without needing to modify or create a newSnapEntity
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 networkedvelocity
variable, simply call itnet_velocity
. But if you want to only change it in accordance with network updates based on your own logic, you can create anet_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?