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

fix(mechanics): Properly reset player flagship after death #10128

Merged
merged 8 commits into from
Jun 2, 2024

Conversation

tibetiroka
Copy link
Member

@tibetiroka tibetiroka commented May 30, 2024

Bug fix

This PR addresses the bug described in issue #10125

Summary

Resetting the player flagship seems to fix most issues, except:

  • the ammo counter is still visible (but cannot be interacted with)
  • the trading panel is still available

Testing Done

Tested with a mission that kills the player on a planet. Needs testing with mid-flight death.

@tibetiroka tibetiroka added bug Something in the game is not behaving as intended mechanics Things dealing with the mechanics & code of how the game works labels May 30, 2024
@TomGoodIdea
Copy link
Member

You can't depart wihtout a flagship, but you still have access to some other options, like the trading panel
Screenshot_20240530_210444

Copy link
Contributor

@RisingLeaf RisingLeaf left a comment

Choose a reason for hiding this comment

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

The other two may be done in a separate PR, maybe we just drop the planet panel on death

@TomGoodIdea
Copy link
Member

The other two may be done in a separate PR, maybe we just drop the planet panel on death

I think destroying the planet panel on death would fix this issue as well, because without a planet panel the player wouldn't be able to depart

@TomGoodIdea
Copy link
Member

Simple flagship reset isn't solving anything, here's a screenshot from the outfitter:
1
And here's your save from #10125, but moved the player to Luna:
Matthew_WeaverGhost.txt
There's a shipyard on Luna, so you can just go there, buy a new ship and depart, but you're still considered dead.

@tibetiroka
Copy link
Member Author

tibetiroka commented May 31, 2024

You can't depart wihtout a flagship, but you still have access to some other options, like the trading panel

I can't reproduce that. When I die on a planet, I don't see the trading panel, neither in this PR on on master.

Nvm, I think I got it.

@tibetiroka
Copy link
Member Author

I think I got all the necessary UI fixes in as well. Do you have a save file for a mid-flight death for testing?

@RisingLeaf
Copy link
Contributor

I would still prefer if you popped the planet panel instead of blocking input

@tibetiroka
Copy link
Member Author

Looks like I need to do both.

source/PlanetPanel.cpp Outdated Show resolved Hide resolved
source/MainPanel.cpp Show resolved Hide resolved
source/MainPanel.cpp Show resolved Hide resolved
source/PlayerInfo.cpp Show resolved Hide resolved
tibetiroka and others added 2 commits May 31, 2024 17:56
Co-authored-by: Rising Leaf <85687254+RisingLeaf@users.noreply.github.com>
Copy link
Contributor

@RisingLeaf RisingLeaf left a comment

Choose a reason for hiding this comment

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

I am not able to test, but looks good

@tibetiroka
Copy link
Member Author

Tested and working with mid-flight death as well.

source/MainPanel.cpp Show resolved Hide resolved
source/PlanetPanel.cpp Show resolved Hide resolved
@tibetiroka tibetiroka linked an issue Jun 1, 2024 that may be closed by this pull request
1 task
Copy link
Member

@TomGoodIdea TomGoodIdea left a comment

Choose a reason for hiding this comment

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

Looks like it finally works

@TomGoodIdea TomGoodIdea added the waiting on dev A developer needs to do something, e.g. reviewing, merging, resolving disputes, etc. label Jun 1, 2024
@Amazinite Amazinite removed the waiting on dev A developer needs to do something, e.g. reviewing, merging, resolving disputes, etc. label Jun 2, 2024
@Amazinite Amazinite merged commit 2ee2291 into endless-sky:master Jun 2, 2024
18 checks passed
@SomeTroglodyte
Copy link

SomeTroglodyte commented Jun 8, 2024

I don't see #9718 linked, but I suppose #10162 deals with that regression? Edit: Confirmed, it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something in the game is not behaving as intended mechanics Things dealing with the mechanics & code of how the game works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

die does not block player interactions
5 participants