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 "Not in a step" exception with battle calc. #10689

Merged
merged 2 commits into from
Jun 23, 2022

Conversation

asvitkine
Copy link
Contributor

@asvitkine asvitkine commented Jun 18, 2022

Change Summary & Additional Notes

Fix "Not in a step" exception w/ battle calc.

This fixes #10673, which can sometimes happen due to the cloned game state being used for battle calc simulation having a blank history. This change ensures that the cleared history has a round and step started.

Release Note

@asvitkine
Copy link
Contributor Author

Hmm, not sure what's going on with the unit tests checker.

@asvitkine asvitkine force-pushed the hist branch 2 times, most recently from 1eeb1a6 to 5dfe276 Compare June 19, 2022 13:19
@asvitkine asvitkine changed the title Fix "Not in a step" exception w/ battle calc. Fix "Not in a step" exception with battle calc. Jun 19, 2022
@asvitkine
Copy link
Contributor Author

Still not sure why this makes Unit Tests hang.

@asvitkine
Copy link
Contributor Author

OK, figured out the test hang issue. The new code in resetHistory() was trying to acquire a write lock, while in a block that was holding a read lock, causing a dead lock. Surprises there's no check in the lock code that verifies this.

Fixed by grabbing a write lock in the outer code. This should have been the case anyway, since state is being changed (e.g. swapping history, etc).

@codecov
Copy link

codecov bot commented Jun 23, 2022

Codecov Report

Merging #10689 (26006c9) into master (0f52f7e) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master   #10689      +/-   ##
============================================
+ Coverage     27.66%   27.75%   +0.08%     
- Complexity     7967     7993      +26     
============================================
  Files          1210     1210              
  Lines         77805    77811       +6     
  Branches      10578    10578              
============================================
+ Hits          21526    21596      +70     
+ Misses        54345    54273      -72     
- Partials       1934     1942       +8     
Impacted Files Coverage Δ
...main/java/games/strategy/engine/data/GameData.java 62.96% <100.00%> (+2.74%) ⬆️
...mes/strategy/engine/framework/GameDataManager.java 45.45% <100.00%> (ø)
...main/java/org/triplea/game/startup/SetupModel.java 66.66% <0.00%> (-33.34%) ⬇️
.../src/main/java/games/strategy/net/nio/Decoder.java 74.13% <0.00%> (-5.18%) ⬇️
.../changefactory/units/UnitDamageReceivedChange.java 90.47% <0.00%> (-0.44%) ⬇️
...va/games/strategy/engine/framework/ServerGame.java 0.55% <0.00%> (-0.01%) ⬇️
...a/org/triplea/game/server/HeadlessServerSetup.java 0.00% <0.00%> (ø)
...ategy/engine/framework/startup/mc/ClientModel.java 0.88% <0.00%> (ø)
...ategy/engine/framework/startup/mc/ServerModel.java 0.00% <0.00%> (ø)
.../engine/framework/startup/ui/ServerSetupPanel.java 0.00% <0.00%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f52f7e...26006c9. Read the comment docs.

@asvitkine asvitkine merged commit 1f65bc3 into triplea-game:master Jun 23, 2022
@asvitkine asvitkine deleted the hist branch July 27, 2023 13:26
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.

AiGameTest > testAiGame sometimes hits "Not in a step, but trying to add event"
1 participant