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

Entity System Refresh #1940

Merged
merged 8 commits into from
Dec 26, 2022
Merged

Entity System Refresh #1940

merged 8 commits into from
Dec 26, 2022

Conversation

Sunrunner37
Copy link
Member

@Sunrunner37 Sunrunner37 commented Dec 25, 2022

This is a long pending update from several years ago that was hanging on another branch. I took the time to update it for the latest code and it seems to be parity with the current entity state (including Jannify's missing door). This PR has a dependency on PR #1937 just because it actually gets entities to work in the new update (for now ignore those from the diff).

In the current world, Nitrox has an entity system that is mostly world objects (such as fish) but it also includes some other 'things' (such as prefab components, like a button or a dropped, serialized item). The entity model is entirely rigid and assumes that all of these have world positions, serialized data, or can be added to a water park; thus, we always transmit all this data for every entity and every update. In parallel, Nitrox has implemented redundant indexing and metadata management systems, examples include base pieces, vehicles, dropped items, containers. All of these are just 'things' in the world but maintain their own dictionaries and processors to update. Nitrox has also been lazy about some of this management, serializing entire gameobjects to transmit (such as dropped items) that bloating network usage, making syncing certain actions impossible (seaglide battery change), and even break entirely in the new update.

In this PR I introduce a single way to unify redundant systems under the concept of 'Entity'. Entities are meant to be subclassed but have commonalities like an id, metadata, and children. This will dramatically simplify the architecture of Nitrox as we don't need separate packets, processor, or data storage for every type of 'thing' that changes state. Instead, entities and their metadata processors will do most of the heavy lifting. If a vehicle changes position, the system sends an entity update. If a button is pressed and changes colors, entity update. If fuel is added to a reactor, entity metadata update. You get the picture.

The first usage of this new system is to convert the old entities into new ones. I've introduced a 'WorldEntity', which satisfies the previous purpose but also allows programmers to understand that it is a physical item that exists. Similarly, I've introduced a 'PrefabChildEntity', which solves for children of WorldEntities that are spawned inside a prefab and only need their ID updated (such as a keypad to a door); in this case, there is no need to transmit the other 8 fields in WorldEntity. Much more can be added, and this large update is a good time to do it.

I've done some level of testing but will obviously need help making this successful. Below is a screenshot showing that fragments, fish, ores all spawn and move correctly.

image

NitroxServer/GameLogic/Entities/EntityRegistry.cs Outdated Show resolved Hide resolved

public IWorldEntitySpawner ResolveEntitySpawner(WorldEntity entity)
{
if (entity.ClassId == "55d7ab35-de97-4d95-af6c-ac8d03bb54ca")
Copy link
Member

@dartasen dartasen Dec 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to detect properly that it is a CellRoot spawner ?
Or it is the only "easy" way

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean with properly? The classId is a unique identifier which ties the entity to a particular prefab.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be a unique ID, but it still hardcoded in our code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's hardcoded but will probably never change. It survived the SN update after all.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree with dartasen that it might be problematic should this value change. I assume there is no way for us to get this programmically from subnautica itself?

Copy link
Member

@Jannify Jannify Dec 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is probably but just with another magic string which can change. ClassId looks to be the most reliable option.

Copy link
Member Author

@Sunrunner37 Sunrunner37 Dec 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought: this might be a good way to utilize the new entity system. We can check for top-level items that have no parent and initialize them as a CellRootEntity (which doesn't have all the fancy features of WorldEntity). Then in CellEntities packet was can send a List<CellRootEntity> which have their WorldEntity as children.

However, since this code is pre-existing, and simply moved from one place to another, I will explore such a refactor as a follow-up.

NitroxServer/Server.cs Show resolved Hide resolved
Copy link
Collaborator

@Coding-Hen Coding-Hen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks cleaner for the entity system. I like the way this is implemented.

I think a rebase might be good to remove a few of the changes which are already in master.


public IWorldEntitySpawner ResolveEntitySpawner(WorldEntity entity)
{
if (entity.ClassId == "55d7ab35-de97-4d95-af6c-ac8d03bb54ca")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree with dartasen that it might be problematic should this value change. I assume there is no way for us to get this programmically from subnautica itself?

@Sunrunner37 Sunrunner37 force-pushed the entity_refactoring branch 2 times, most recently from ba2ffa3 to 28e456e Compare December 26, 2022 05:49
…y, this facilitates much smaller packet sizes in prefab dense zones such as the aurora. From a code standpoint, this offers separation of concerns and opens up opportunity to unify redundant systems such as items (which are essentially entities that also need persisted metadata). This also removes the concept of storing serialized game objects (such as on drop) because they are a lazy way of not implementing the appropriate metadata payload.

Known regressions:
- Ping no longer updating (need to convert this to entity metadata system)
- Dropped items resetting state (such as a battery - again need to use entity metadata system)
Known regressions:
- Ping no longer updating (need to convert this to entity metadata system)
- Dropped items resetting state (such as a battery - again need to use entity metadata system)
@Sunrunner37
Copy link
Member Author

The PR is ready for review again. I've implemented the requested changes and merged Jannify's changes as well (with just some minor tweaks). We have doors!

image

@Sunrunner37 Sunrunner37 merged commit 915ff79 into master Dec 26, 2022
@Sunrunner37 Sunrunner37 deleted the entity_refactoring branch December 26, 2022 22:02
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

5 participants