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

Serialize the player's inventory in save files #431

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

patowen
Copy link
Collaborator

@patowen patowen commented Oct 5, 2024

This completes part of #428.

Allows users to exit and re-open Hypermine while keeping the contents of their inventory, as previously, the inventory was cleared every time the game was loaded.

@patowen patowen requested a review from Ralith October 5, 2024 20:36
server/src/sim.rs Outdated Show resolved Hide resolved
)
.unwrap();
entities.push(repr);
let reprs = self.snapshot_entity_and_children(entity);
Copy link
Owner

Choose a reason for hiding this comment

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

Are child entities not also in graph_entities?

Copy link
Collaborator Author

@patowen patowen Oct 12, 2024

Choose a reason for hiding this comment

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

Not currently, although given how tricky this implementation is, I would consider this to be a mistake on my part.

There's a distinction between entities that have an inherent position (which is managed with a Position component) and nodes that don't, such as inventory items. For the latter, they belong to an entity that does have a position.

I had decided that graph_entities would only contain entities that have a position, but I have a good reason to second-guess that, since that doesn't correspond with how save files are organized at all. I'll go ahead and fix this before my next push to this branch.

EDIT: Actually, I might have to think about this some more.

Copy link
Collaborator Author

@patowen patowen Nov 27, 2024

Choose a reason for hiding this comment

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

I've updated the PR so that child entities are now in graph_entities. To accomplish this (what I believe to be) the most understandable way possible, I've made NodeId into its own component intrinsically linked with graph_entities so that inventory items can be associated with a NodeId without needing a Position.

This is helpful because Position has a bunch of other functionality, such as syncing with clients, so separating this functionality is important.

I'll resolve this thread for now, as child entities now are in graph_entities, but the resolution may be controversial, and I'm definitely open to feedback.

server/src/sim.rs Outdated Show resolved Hide resolved
@patowen patowen force-pushed the save-inventory branch 2 times, most recently from c1b6e2a to f59d7d3 Compare November 27, 2024 21:55
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.

2 participants