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

Save game dialog does not appear #4755

Open
cwyss opened this issue Dec 14, 2019 · 20 comments · May be fixed by #4767
Open

Save game dialog does not appear #4755

cwyss opened this issue Dec 14, 2019 · 20 comments · May be fixed by #4767
Assignees

Comments

@cwyss
Copy link
Contributor

@cwyss cwyss commented Dec 14, 2019

Observed behaviour

The save game dialog does not open when pressing the save button, i.e. flight UI -> Esc -> "Save" button.
After pressing the save button once, also Load Game from the main menu stops working.
quicksave via Ctrl-F9 works fine.

My pioneer version (and OS):
d0f2655 (current master)

last bug free:
d016f7a (Nov 16 2019)

@impaktor

This comment has been minimized.

Copy link
Member

@impaktor impaktor commented Dec 14, 2019

Yep, it's a known bug, I hope it will be short lived, as it's pretty serious for user experience.
Thanks for reporting!

@cwyss

This comment has been minimized.

Copy link
Contributor Author

@cwyss cwyss commented Dec 14, 2019

Thanks for the info.

I also noticed just now that the dialog box to change a key binding opens behind the options dialog box. But this is probably also known already... :)

@impaktor

This comment has been minimized.

Copy link
Member

@impaktor impaktor commented Dec 19, 2019

I assume the bug was introduced in f04ea1e, @vakhoir do you have time to look into this?

Gliese852 added a commit to Gliese852/pioneer that referenced this issue Jan 2, 2020
@impaktor

This comment has been minimized.

Copy link
Member

@impaktor impaktor commented Jan 5, 2020

....or if you @cwyss have time to look at this, that would be nice. Seems like a pretty major serious bug.

@vakhoir

This comment has been minimized.

Copy link
Contributor

@vakhoir vakhoir commented Jan 9, 2020

@vakhoir do you have time to look into this?

I finally do!
Looking into it now

@impaktor

This comment has been minimized.

Copy link
Member

@impaktor impaktor commented Jan 9, 2020

Great! Do note, there's a PR for it, #4755, but author seems to indicate it's not working as expected.

I think @Web-eWorks said he knew what was amiss with this bug:

<sturnclaw> [08:51:30] yeah, the save issue is just the code in pigui.lua
            being designed for only one modal
<sturnclaw> [08:52:02] I can rip that out and rewrite it in an afternoon, but
            I haven't been able to *think* much less work on Pioneer for the
            past week or so
@Web-eWorks

This comment has been minimized.

Copy link
Member

@Web-eWorks Web-eWorks commented Jan 9, 2020

Hmm, on second thought we don't seem to be wrapping the ImGui::BeginPopupModal() in any custom code, so it may just be an incorrect use of ui.openPopup() / ui.closeCurrentPopup(); I'd recommend investigating the imgui docs and looking at the code in imgui.cpp and related files...

@vakhoir

This comment has been minimized.

Copy link
Contributor

@vakhoir vakhoir commented Jan 9, 2020

Yeah, it's fixable in lua, but I have to say I'm not sure what the ImGui people were going for when they decided the separete openPopup from beginPopup, and then having openPopup be called only once on open, when everything else in the UI has to be called once per frame...

Anyways I have something worked out, I'll be testing and polishing it tomorrow.

@vakhoir

This comment has been minimized.

Copy link
Contributor

@vakhoir vakhoir commented Jan 9, 2020

Oh, should've hit refresh

Hmm, on second thought we don't seem to be wrapping the ImGui::BeginPopupModal() in any custom code, so it may just be an incorrect use of ui.openPopup() / ui.closeCurrentPopup();

What do you meant @Web-eWorks ?

It was incorrect in that if you want nested modals, you need to call openPopup after the first BeginPopupModal was called and openPopup should/must be called only once on open, rather than every frame. Makes the whole thing pretty complicated, particularly when each popup is set up by a different script, but I more or less sorted it out.

@Web-eWorks

This comment has been minimized.

Copy link
Member

@Web-eWorks Web-eWorks commented Jan 9, 2020

@vakhoir yeah, that was what I meant - that our current implementation in Lua was mangling the ImGui modal stack. Thanks again for tracking this down!

@vakhoir

This comment has been minimized.

Copy link
Contributor

@vakhoir vakhoir commented Jan 9, 2020

Well, I'm the one that screwed this up in the first place ;)

@vakhoir vakhoir linked a pull request that will close this issue Jan 10, 2020
@vakhoir

This comment has been minimized.

Copy link
Contributor

@vakhoir vakhoir commented Jan 21, 2020

Hm, can anyone confirm that loading a saved games works? I get the following exception thrown:

pioneer: ./pioneer/src/Frame.cpp:191: static FrameId Frame::FromJson(const Json&, Space*, FrameId, double): Assertion '(s_frames.size() - 1) != f->m_thisId.id()' failed.

Can't tell when it started, probably somewhere around #4669, #4745, or #4754. Still works fine on my (rather old) branch https://github.com/vakhoir/pioneer/tree/feature/rescale_ui

@cwyss

This comment has been minimized.

Copy link
Contributor Author

@cwyss cwyss commented Jan 21, 2020

@vakhoir

Hm, can anyone confirm that loading a saved games works? I get the following exception thrown:

Never had such a problem, as long as I did not try to load very old saved games, i.e., which were saved with a pioneer version that is much older than the one used for loading. In particular, I get no problems doing ctrl-f9 and then loading _quicksave again in 0881148

@vakhoir

This comment has been minimized.

Copy link
Contributor

@vakhoir vakhoir commented Jan 21, 2020

I made sure to save a new game during the test, and I get the same result testing from the commit you specified.

I'm suspecting this is some compiler/OS weirdness. Anyone else using GCC on Ubuntu?

@Gliese852

This comment has been minimized.

Copy link
Contributor

@Gliese852 Gliese852 commented Jan 21, 2020

I am using GCC on Ubuntu. Installed this branch. The problem with loading both new saves and slightly older ones (about a month) is not observed.

@vakhoir

This comment has been minimized.

Copy link
Contributor

@vakhoir vakhoir commented Jan 21, 2020

Huh... well, that's frustrating. It's 100% reproducible for me.

@Web-eWorks

This comment has been minimized.

Copy link
Member

@Web-eWorks Web-eWorks commented Jan 21, 2020

@Gliese852 can you try running ./bootstrap -DCMAKE_BUILD_TYPE=Debug and recompiling? Sounds like this is an assertion that's being triggered but isn't throwing because you're compiled in Release mode...

We've had intermittent issues with this frame code, so I'll take a closer look at it later on this week.

@Gliese852

This comment has been minimized.

Copy link
Contributor

@Gliese852 Gliese852 commented Jan 21, 2020

pioneer: ~/src/Gliese852/pioneer/src/Frame.cpp:191: static FrameId Frame::FromJson(const Json&, Space*, 
FrameId, double): Assertion `(s_frames.size() - 1) != f->m_thisId.id()' failed.
Aborted (core dumped)
@Web-eWorks

This comment has been minimized.

Copy link
Member

@Web-eWorks Web-eWorks commented Jan 22, 2020

Alright, good to know. I'll dig into the save / load code (as well as a saved game) and see what the issue is; my guess is that we still have some incorrect logic related to frame ordering.

Can anyone confirm that loading a save causes any issues? Obviously something's a little wonky here but my guess is that this is mostly a harmless assert()

@vakhoir

This comment has been minimized.

Copy link
Contributor

@vakhoir vakhoir commented Jan 23, 2020

Can anyone confirm that loading a save causes any issues? Obviously something's a little wonky here but my guess is that this is mostly a harmless assert()

Yeah, everything works fine when it's commented out.

@Web-eWorks Web-eWorks self-assigned this Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.