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: Save objects states in Obj Manager to keep them btwn rooms #554

Merged
merged 1 commit into from
Apr 23, 2022

Conversation

StraToN
Copy link
Collaborator

@StraToN StraToN commented Mar 24, 2022

@BHSDuncan
Copy link
Collaborator

How are we going to merge this? If we put it into develop before we merge the "instant transition" PR, it's going to cause merge conflicts, especially given how very much different the object and room managers have become.

How do we want to handle this?

dploeger
dploeger previously approved these changes Mar 28, 2022
Copy link
Collaborator

@dploeger dploeger left a comment

Choose a reason for hiding this comment

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

Thanks! 🎁

@dploeger
Copy link
Collaborator

Sorry, didn't read your comment, @BHSDuncan. Maybe you could incorporate this change in your branch then? It isn't quite big.

@StraToN
Copy link
Collaborator Author

StraToN commented Mar 29, 2022

@BHSDuncan Agree with @dploeger . I'm fine if you cherry-pick this commit into #515 and we close this one.

@StraToN
Copy link
Collaborator Author

StraToN commented Apr 5, 2022

Status: waiting for #515 to be merged. Will have to be rebased after that.

@StraToN
Copy link
Collaborator Author

StraToN commented Apr 11, 2022

To test:

  • checkout branch "room6" from @balloonpopper
  • rebase this branch on origin/develop
  • If you want to see the issue, test (apply scenario below)
  • apply this PR onto the resulting branch
  • run

Scenario:

  1. Access room06 -> notice that right door is closed
  2. exit room06 to room05
  3. Get back to room06
    -> if PR not applied, notice that right door is now open
    -> else, notice that right door is still closed

@balloonpopper
Copy link
Collaborator

The state was preserved in my testing - the door stays "closed" with the fix applied.
Other issues were found in the transition between these rooms (as dicsussed in the channel) but they seem unrelated to this PR.

@BHSDuncan
Copy link
Collaborator

The state was preserved in my testing - the door stays "closed" with the fix applied. Other issues were found in the transition between these rooms (as dicsussed in the channel) but they seem unrelated to this PR.

Can you also maybe check if it's preserved across saving and loading? It should be based on what I see, but best to test. :)

@StraToN StraToN mentioned this pull request Apr 12, 2022
balloonpopper
balloonpopper previously approved these changes Apr 12, 2022
@balloonpopper
Copy link
Collaborator

balloonpopper commented Apr 12, 2022

Can you also maybe check if it's preserved across saving and loading? It should be based on what I see, but best to test. :)

FYI : I was asked to hold off on save testing while @StraToN does some more work on the save system.

@StraToN StraToN dismissed stale reviews from balloonpopper and dploeger April 13, 2022 22:55

Code has changed importantly with last commit

@StraToN StraToN marked this pull request as draft April 21, 2022 16:47
@StraToN StraToN marked this pull request as ready for review April 21, 2022 20:49
@StraToN StraToN changed the title feat: Saves objects states in Objects Manager to keep them between rooms feat: Save objects states in Obj Manager to keep them btwn rooms Apr 23, 2022
@StraToN StraToN merged commit 4b2b6f5 into develop Apr 23, 2022
@StraToN StraToN deleted the issue-193 branch April 23, 2022 16: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.

states aren't preserved over room transitions
4 participants