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

Mapgen missing key detection #38063

Merged

Conversation

jbytheway
Copy link
Contributor

Summary

SUMMARY: Infrastructure "Detect mapgen keys which are used but not defined, to help catch typos in maps"

Purpose of change

As demonstrated by #37845 and #37839, it is easy for mappers to accidentally put typos into their maps or forget to define particular keys they use in their maps. Having fixed those issues, we want to prevent similar issues from occurring in the future.

Describe the solution

When parsing the rows entry from json mapgen, verify that every character used has some definition, except that . and are permitted to have no definition provided fill_ter is being used, or the entry defines nested mapgen.

The error is fatal, but only occurs in test_mode.

I also refactored some of the surrounding code for greater clarity, including renaming the (mysterious to me) qualifies variable.

Describe alternatives you've considered

Some mods still contain many mapgen errors, so it doesn't seem feasible to make this a non-test-mode error yet, because it would be annoying for players using those mods (even if it were made non-fatal), but once those mods are fixed or removed from the main repo, that should be changed.

Testing

It was with this code that I found all the errors fixed in #37845 and #37839, so that tested it fairly well.

Additional context

This was originally inspired by me implementing a feature addition for Unicode mapgen keys. Once this is merged, I plan to PR that.

This error is fatal, and not all mods are clean with respect to it, so
leaving it in regular play would break a bunch of people's games.  (Even
if it were non-fatal, it would be super annoying for players to click
through many debugmsgs).

Make it test-mode only to avoid that.

Unfortunately, that means that mappers who don't run the tests won't
find out about their mapgen typos until they PR them, but I think it's
the best we can do for now.
The undefined key error has two special case characters.  This needs to
be documented.
@ZhilkinSerg ZhilkinSerg added <Documentation> Design documents, internal info, guides and help. [C++] Changes (can be) made in C++. Previously named `Code` [Markdown] Markdown issues and PRs Code: Build Issues regarding different builds and build environments Code: Tests Measurement, self-control, statistics, balancing. Map / Mapgen Overmap, Mapgen, Map extras, Map display labels Feb 16, 2020
@Rivet-the-Zombie Rivet-the-Zombie merged commit ac0b238 into CleverRaven:master Feb 17, 2020
@jbytheway jbytheway deleted the mapgen_missing_key_detection branch February 19, 2020 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Build Issues regarding different builds and build environments Code: Tests Measurement, self-control, statistics, balancing. <Documentation> Design documents, internal info, guides and help. Map / Mapgen Overmap, Mapgen, Map extras, Map display [Markdown] Markdown issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants