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

Maps crashing on Location::processTimers #635

Open
667bdrm opened this issue Jul 14, 2020 · 4 comments
Open

Maps crashing on Location::processTimers #635

667bdrm opened this issue Jul 14, 2020 · 4 comments

Comments

@667bdrm
Copy link
Contributor

667bdrm commented Jul 14, 2020

denbus1, denbus2, denres1, newrgo, newrst, modmain, reddown, newrvb, newrcs crashing

See attached debug output and gdb trace
denbus1-crash-gdb.txt

Related issues: #271, #418, #430

Expected Behavior

Game should not crash

Steps to Reproduce

  1. Set one of the following maps as initial location denbus1, denbus2, denres1
  2. Walk through the map some time, game will crash

System

  • Falltergeist: 0.4.0 develop branch (3b1c620)
  • Operating System: Ubuntu 20.04 LTS
  • Fallout: 1.02 US
@JanSimek
Copy link
Contributor

The same crash happens on the newr2.map and I attached a patch that fixes the crash to #271.

diff --git a/src/State/Location.cpp b/src/State/Location.cpp
index e59766ac..a46dfa66 100644
--- a/src/State/Location.cpp
+++ b/src/State/Location.cpp
@@ -518,8 +518,9 @@ namespace Falltergeist
             _ambientSfxTimer.think(deltaTime);
 
             for (auto it = _timerEvents.begin(); it != _timerEvents.end();) {
-                it->timer.think(deltaTime);
-                if (!it->timer.enabled()) {
+                TimerEvent currentEvent = *it;
+                currentEvent.timer.think(deltaTime);
+                if (!currentEvent.timer.enabled()) {
                     it = _timerEvents.erase(it);
                 } else ++it;
             }

I have no idea why that works. Basically, I was just testing any possible solution I could find on StackOverflow and this one worked.

@667bdrm
Copy link
Contributor Author

667bdrm commented Jul 14, 2020

It helps partially if I starting from denbus2 or denres1 - there are no crashes , but when I going from there to denbus1 through exit grids (or just starting from denbus1), the crash on erase still happens.

@667bdrm
Copy link
Contributor Author

667bdrm commented Jul 16, 2020

I have asked about that our C++ developer, he told that it is a bad practice to remove list elements during iteration. He recommends to collect element positions then remove it in separate loop from the last to first (because if you begin from first the positions of next will be changed after each removal). But when I have played with that code looks like ants animation in artemple has been broken so needs to be careful with processTimers patching - the engine could be easy broken by the changes.

@667bdrm 667bdrm changed the title Den maps crashing Maps crashing on Location::processTimers Jul 18, 2020
@667bdrm
Copy link
Contributor Author

667bdrm commented Jul 28, 2020

Tried to use shared_ptr for storing _timerEvents with the same result

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

No branches or pull requests

2 participants