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

Guest world #773

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Guest world #773

wants to merge 2 commits into from

Conversation

philpax
Copy link
Contributor

@philpax philpax commented Aug 31, 2023

This is an attempt to switch the guest over to using a World struct instead of global functions to help resolve #725 and #230 and to enable #71 in the future.

However, there are a few blocking issues:

  • The AnimationNode Clone and Drop implementations rely on modifying the ECS globally, which doesn't work when there's a World floating around. I've disabled this for now just to make it compile, but that's not a workable solution.
  • Anything involving callbacks requires passing the World through, including event subscriptions.
  • Every bit of Ambient code will need to be updated.

I'm leaving this as a draft PR to pick up later, but it seems like something we should do at some point.

@FredrikNoren
Copy link
Contributor

@philpax Hm yeah.. I'm not entirely happy with AnimationNode, and it feels like this might actually expose that the design isn't that good. I'll take a look if I can figure out how to do the animation stuff differently

@FredrikNoren FredrikNoren mentioned this pull request Sep 5, 2023
@philpax
Copy link
Contributor Author

philpax commented Sep 5, 2023

With the recent animation API changes, this is now viable again. I'll revisit once I'm done with other work.

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.

Add object-oriented EntityId methods
2 participants