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

Cleanup filter & region parsing & validation #1053

Merged
merged 1 commit into from
Sep 11, 2022
Merged

Conversation

Pablete1234
Copy link
Member

@Pablete1234 Pablete1234 commented Sep 9, 2022

Originally started to fix the /modes list command, ended up fixing way too many more things. This includes a more proper fix for the quick-and-dirty if-check added in #1050.

Monument Mode fixes:

  • /mode list command sorting was still broken (initially tackled in Fix mode commands with filtered modes #1048, still broken in some conditions)
  • Added current page and max page to /mode list.
  • Migrated mode list pagination from legacy string-based to modern component-based.
  • Tweaked ordering of mode list, started modes (or unfiltered) always first, then sorted by time, lastly sorted alphabetically.
  • Modes' filter must now be dynamic & react to match changes, validated at parse time

Filter/region validation/parsing fixes:

  • Adds proper dynamic filter validation, features which require a filter to be dynamic will verify that at parse time
  • Validations will run after references have been resolved, this prevents unresolved-forward-references trying to perform a validation they can't yet ensure
  • Unified util methods to parse regions and filters in to an XMLParser utility, that both extend. This makes a handful of unified parseProperty methods available in both RegionParser and FilterParser, with overloads for defaults, required, and validations.
  • Removed the legacy String... names from region parsing, most of the times a single name is used, in the few places where multiple names are allowed they were replaced with calls that use Node.fromChildOrAttr(el, name1, name2) or similar.
  • Removed dynamic filter listeners without a scope, those would listen on Filterable, which is essentially a guarantee that you'll break something. Filters need a scope to filter, they can't just filter "anything". If you're fine with "anything" simply use Filterables.scope(filter) to find what the least-specific context the filter supports listening for.
  • Added line numbers to xml errors for attributes. Before attributes would only say attribute X in element Y, now they also include line numbers of the element.

Note: No PGM behavior should have changed at all. No XML changes, no run-time changes. Only more strict validation for invalid usages of dynamic filters (eg: trying to use a non-dynamic filter in a place that requires a dynamic).

Note: This has been tested with main OCC map repositories, public and private, and has not caused any new map xml error within them except for the ones i manually introduced for testing, meaning this should already be in a stable state.

@Pablete1234 Pablete1234 added the bug Something isn't working label Sep 9, 2022
@Pablete1234 Pablete1234 added the ready PR is ready to merge label Sep 10, 2022
@Electroid Electroid merged commit db22ec9 into dev Sep 11, 2022
@Electroid Electroid deleted the fix-monument-cmd branch September 11, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready PR is ready to merge
Development

Successfully merging this pull request may close these issues.

2 participants