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

feat: Add Session commands to modify sessions from game code #379

Merged
merged 2 commits into from
Apr 14, 2024

Conversation

MaxCWhitehead
Copy link
Collaborator

Add SessionCommands which may be inserted as shared resource. Runs after session execution, before the "after_session" GameSystems.

One thing I don't like about this is it only works as shared resource, if user does not insert shared resource, and tries to do ResMutInit, they run into trouble. Not sure what the best way to avoid that is.

@zicklag
Copy link
Member

zicklag commented Apr 5, 2024

What if we just added the queue as a field to Session?

@MaxCWhitehead
Copy link
Collaborator Author

MaxCWhitehead commented Apr 5, 2024

What if we just added the queue as a field to Session?

This was my first attempt: I think I found that the current session did not have the values in queue - but reviewing code again I am thinking that the game session is not inside its session lists when currently running (I might've done if let Some(game_session)... game_session.add_session_command) and not added it (if it was indeed None from within system in game session).

Or the issue is that the session I added it to was not accessed correctly in the context of how we swap sessions in/out of resource + remove and reinsert current session. Some sort of bug like that - but maybe can make that work.

I do like that this removes burden of SessionCommands shared resource init. Will take another look and follow up on if there is a blocker here or if I just got it wrong.

@MaxCWhitehead
Copy link
Collaborator Author

@zicklag The issue with putting command queue on Session is that the current running session is not in the Sessions resource. Game session cannot insert session command into its own session, only other sessions. So probably makes sense to have session commands exist outside of sessions themselves.

@zicklag
Copy link
Member

zicklag commented Apr 6, 2024

But the current running session doesn't need to be in the sessions resource when the command is inserted, because the command is only run after the current session is put back into the sessions resource.

@MaxCWhitehead
Copy link
Collaborator Author

But the current running session doesn't need to be in the sessions resource when the command is inserted, because the command is only run after the current session is put back into the sessions resource.

I'm not understanding how this is to be used though:
Inside a system in game session, if I do this:

// sessions = ResMut<Sessions>
if let Some(game_session) = sessions.get_mut(SessionNames::Game) {
    game_sessions.session_commands.push_back(<command>);
}

This does nothing because game session is not found in sessions, due to it being removed during execution.

@zicklag
Copy link
Member

zicklag commented Apr 6, 2024

Oh, I'm saying that we put the queue in Sessions, not in Session. So instead it'd be:

// sessions = ResMut<Sessions>
sessions.commands.push_back(<command>);

@MaxCWhitehead
Copy link
Collaborator Author

@zicklag updated so commands are on Sessions.

@MaxCWhitehead MaxCWhitehead added this pull request to the merge queue Apr 14, 2024
Merged via the queue into fishfolk:main with commit 8be1965 Apr 14, 2024
10 checks passed
@MaxCWhitehead MaxCWhitehead deleted the session_commansd branch April 14, 2024 16:22
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

2 participants