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

Migrate overmap specials #53702

Merged
merged 5 commits into from
Dec 25, 2021
Merged

Conversation

dseguin
Copy link
Member

@dseguin dseguin commented Dec 23, 2021

Summary

None

Purpose of change

Fixes #53224.
Fixes #53423.
Fixes #51856.

Describe the solution

I've added a new type "overmap_special_migration" to handle overmap specials that have been migrated or removed:

{
  "type": "overmap_special_migration",
  "id": "Military Bunker",
  "new_id": "military_bunker"
},
{
  "type": "overmap_special_migration",
  "id": "basin",
  "//": "Removed - no new id"
}

TODO:

  • Handle known moved/removed specials (temporary measure)
  • Handle FakeSpecial_ buildings from overmap_specials::create_building_from It turns out it didn't require anything special at all, just non-fake id definitions
  • Jsonify migration so that content creators can migrate specials themselves

Describe alternatives you've considered

I could leave it at hardcoded exceptions, which would work but not optimal.

Testing

Tested loading saves from the linked issues.

Additional context

@dseguin dseguin force-pushed the migrate_om_specials branch from 3d85ebe to ff8ace9 Compare December 23, 2021 06:27
@BrettDong BrettDong added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` (P2 - High) High priority (for ex. important bugfixes) Map / Mapgen Overmap, Mapgen, Map extras, Map display labels Dec 23, 2021
@dseguin dseguin marked this pull request as ready for review December 24, 2021 20:56
@dseguin dseguin requested a review from BrettDong as a code owner December 24, 2021 20:56
@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Dec 24, 2021
@wapcaplet
Copy link
Contributor

Tested OK with my save games (see linked issues). Glad to see these bugs finally resolved!

@dseguin dseguin force-pushed the migrate_om_specials branch from b0bc7d0 to 9a1afca Compare December 24, 2021 21:59
@dseguin dseguin marked this pull request as draft December 24, 2021 22:30
@dseguin
Copy link
Member Author

dseguin commented Dec 24, 2021

There's an issue with one two of the mods using an old id (s_jewelry instead of s_jewelry_shop). One of the tests failed because the consistency check caught it. At least the check is working as intended!

@dseguin dseguin marked this pull request as ready for review December 24, 2021 23:03
@dseguin dseguin requested a review from I-am-Erk as a code owner December 24, 2021 23:03
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 25, 2021
@kevingranade kevingranade merged commit 7e07030 into CleverRaven:master Dec 25, 2021
@dseguin dseguin deleted the migrate_om_specials branch December 25, 2021 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display (P2 - High) High priority (for ex. important bugfixes)
Projects
None yet
4 participants