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

Yet another yet another conflict merge #10

Merged
merged 29 commits into from
Mar 28, 2022

Conversation

a-chancey
Copy link
Owner

Summary

Category "Brief description"

Purpose of change

Describe the solution

Describe alternatives you've considered

Testing

Additional context

dseguin and others added 29 commits March 25, 2022 11:56
* chore: typo hunting ft. codespell

I discovered codespell yesterday while looking at the vim repo
I like it, I use it here

* re-re-re-review all changes

Co-authored-by: Kevin Granade <[email protected]>
Reduce bashing damage
Add multiple flags to reduce description string duplication
Variants were originally added for only guns and magazines, and so
translations were only extracted for those types. Now they can be
applied to all items, so extract for all item types.
@a-chancey a-chancey merged commit 935054c into a-chancey:TailoringRecipeOverhaul-Cotton Mar 28, 2022
a-chancey pushed a commit that referenced this pull request Nov 29, 2023
* Prevents game occasionally seemingly hanging when moving to new submaps

The reason for the previous problem was an infinite loop caused by:
1. `map::spawn_monsters_submap` for-loops the list of `current_submap->spawns`
2. for every spawned monster, it calls `monster::on_load`
3. `monster::on_load` calls `monster::try_reproduce`, which in turn calls
   `map::add_spawn`
4. So a new spawn is added, thus invalidating the iterator used in step 1
5. Undefined behavior caused by using invaliated iterators.

On my compiler (gcc 13.2.0), the above problem had the following effect:
* The reference `spawn_point &i` pointed to something totally different, so
  that in particular, `i.count` had garbage values
* Instead of `i.count` being reasonable values such as `3` or `1`, the above
  undefined behavior made it have values such as `925969776` or `-632214304`
* `i.count` is the upper bound for the inner for-loop in
  `map::spawn_monsters_submap`, so depending on the garbage value, it might
  seem like an infinite loop.

Stacktrace of app when frozen and problem happened:
```
 #0  0x000055a1eaf36dcb in creature_tracker::find(coords::coord_point<tripoint, (coords::origin)1, (coords::scale)0> const&) const ()
 #1  0x000055a1eaf39253 in Creature* creature_tracker::creature_at<Creature>(coords::coord_point<tripoint, (coords::origin)1, (coords::scale)0> const&, bool) ()
 #2  0x000055a1eaf393b5 in Creature* creature_tracker::creature_at<Creature>(tripoint const&, bool) ()
 #3  0x000055a1eb357a53 in map::spawn_monsters_submap(tripoint const&, bool, bool)::{lambda(tripoint const&)#1}::operator()(tripoint const&) const ()
 #4  0x000055a1eb3a976e in random_point(tripoint_range<tripoint> const&, std::function<bool (tripoint const&)> const&) ()
 #5  0x000055a1eb37dd2d in map::spawn_monsters_submap(tripoint const&, bool, bool) ()
 #6  0x000055a1eb37de77 in map::spawn_monsters(bool, bool) ()
 #7  0x000055a1eb093981 in game::update_map(int&, int&, bool) ()
 #8  0x000055a1eb094451 in game::update_map(Character&, bool) ()
 #9  0x000055a1eb09530f in game::place_player(tripoint const&, bool) ()
 #10 0x000055a1eb0b3a0e in game::walk_move(tripoint const&, bool, bool) ()
 #11 0x000055a1ead27490 in avatar_action::move(avatar&, map&, tripoint const&) ()
 #12 0x000055a1eb0f338c in game::do_regular_action(action_id&, avatar&, std::optional<tripoint> const&) ()
 #13 0x000055a1eb0f6e63 in game::handle_action() ()
 CleverRaven#14 0x000055a1eafbd9ea in do_turn() ()
 CleverRaven#15 0x000055a1eaa5ec13 in main ()
```

This commit instead changes the loop in step 1 above so that it explicitly
*not* uses iterators, but instead old-fashioned indexed loop. The intention
with the change is to allow other parts of the code to add items to the vector
`current_submap->spawns` while we are iterating it here. If new items are
added, they will be handled in later steps of the loop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.