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

Add open empty site button on launch menu #143

Closed

Conversation

audrow
Copy link
Contributor

@audrow audrow commented Jul 6, 2023

This adds a button that loads an empty site (which I added to the assets).

When debugging, I was finding myself wanting to work from an empty world so, I would load the demo world and then go File > New. Instead, I just added an empty world to be loaded from the menu. This might be nice for people to start new projects from.

Probably the more correct way to do this is to use CreateNewWorkspace, which I didn't see how to plumb through.

Signed-off-by: Audrow Nash <[email protected]>
demo_office(),
)));
if ui.button("New site").clicked() {
_load_workspace.send(LoadWorkspace::Data(WorkspaceData::Site(empty_site())));
Copy link
Member

Choose a reason for hiding this comment

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

You can always add a new parameter mut _new_workspace: EventWriter<CreateWorkspace> here:

mut _load_workspace: EventWriter<LoadWorkspace>,

and then send on that channel. That would reduce the need for your default world code. We should also merge #131 in first.

Copy link
Member

Choose a reason for hiding this comment

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

I gave it a quick spin and it seems slightly hairy because the CreateNewWorkspace event requires the AppState to be set otherwise it will return an error. Naively setting the AppState before sending the event also seems to not work, probably because the state update was not flushed before the event reader ran. Therefore unless we reorder systems to make sure the app state was correctly set and flushed before the event reader runs the CreateNewWorkspace approach will not work.

Still embedding another demo map is not ideal. I suspect if we set the state and made sure the CreateNewWorkspace events are processed in a separate stage it should work. Another idea could be to just use the site constructor that is used inside the CreateNewWorkspace event handler for this, and just dispatch a LoadSite event.
It's a bit hacky but we need to refactor states soon anyway when upgrading the bevy version so 🤷

@arjo129 is right and we should definitely merge that PR first.
I'll also open up a quick PR to fix CI now that let else support has been added to rustfmt to make it green again.

@audrow
Copy link
Contributor Author

audrow commented Jul 31, 2023

I'm going to close this because it sounds like there are better approaches.

@audrow audrow closed this Jul 31, 2023
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.

3 participants