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

Add MAINLINE_MODS.md to document mod inclusion criteria and procedures #1

Closed
wants to merge 1 commit into from

Conversation

esotericist
Copy link
Owner

Summary

SUMMARY: Infrastructure "Add MAINLINE_MODS.md to document mod inclusion criteria and procedures"

Purpose of change

Provide a clear policy on when mods can be included, and when they should be removed

Describe the solution

Lots of words being typed. Policy establishment

Additional context

Related: CleverRaven#37272

Draft proposal. Open to additions.

In particular, I could really use help formulating the "responsibilities of a maintainer" section

Further, there are additional criteria:

* Content mods need to be providing some kind of curated experience. Either they introduce a set of locations, encounters, equipment, or progression mechanisms that do not fit the style of the core game, or they in some fashion modify the game according to some well-defined concept.
* * Mods which do not qualify as curated experiences: Grab bag mods with no defined purpose (i.e. adding a bunch of randomly guns), "settings" mods that just turn off a possibly undesired but working feature (acid ants).

Choose a reason for hiding this comment

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

Suggested change
* * Mods which do not qualify as curated experiences: Grab bag mods with no defined purpose (i.e. adding a bunch of randomly guns), "settings" mods that just turn off a possibly undesired but working feature (acid ants).
* * Mods which do not qualify as curated experiences: Grab bag mods with no defined purpose (i.e. adding a bunch of random guns), "settings" mods that just turn off a possibly undesired but working feature (acid ants).

@esotericist esotericist added the invalid This doesn't seem right label Jan 21, 2020
esotericist pushed a commit that referenced this pull request Apr 23, 2020
esotericist pushed a commit that referenced this pull request Apr 23, 2020
esotericist pushed a commit that referenced this pull request Oct 13, 2020
esotericist pushed a commit that referenced this pull request Nov 13, 2023
be more specific about what writing "Fixes: #1" does, it closes the
linked issue if your PR is merged

comment commands regex can get... elaborate add a comment with an
example where it gets complex, also indicate the maintainer and curator
of the comment commands action
esotericist pushed a commit that referenced this pull request Dec 2, 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) ()
 CleverRaven#7  0x000055a1eb093981 in game::update_map(int&, int&, bool) ()
 CleverRaven#8  0x000055a1eb094451 in game::update_map(Character&, bool) ()
 CleverRaven#9  0x000055a1eb09530f in game::place_player(tripoint const&, bool) ()
 CleverRaven#10 0x000055a1eb0b3a0e in game::walk_move(tripoint const&, bool, bool) ()
 CleverRaven#11 0x000055a1ead27490 in avatar_action::move(avatar&, map&, tripoint const&) ()
 CleverRaven#12 0x000055a1eb0f338c in game::do_regular_action(action_id&, avatar&, std::optional<tripoint> const&) ()
 CleverRaven#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.
GuardianDll pushed a commit that referenced this pull request Dec 21, 2024
Attempt to fix merge conflicts
GuardianDll pushed a commit that referenced this pull request Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants