Feature request: Warning suppression for Mod validation

[hackedpassword]

Category: Doesn't Hurt To Ask <3

This is a follow up to #10355

While warning about unsupported features via modchecker is useful and practical, the introduction of these recent warnings is excessive, imo, based upon the following rationale.

Tighten RulesetValidator for Terrain introduced the following warnings into mod Z2, as expected:

CoastTile turns into terrain Force which is not a base terrain!
OceanTile turns into terrain Force which is not a base terrain!
Bridge1ocean turns into terrain Force which is not a base terrain!
Bridge2ocean turns into terrain Force which is not a base terrain!
Bridge1coast turns into terrain Force which is not a base terrain!
Bridge2coast turns into terrain Force which is not a base terrain!
Bridge3coast turns into terrain Force which is not a base terrain!
Bridge3 turns into terrain Force which is not a base terrain!
BridgeFL occurs on natural wonder CoastTile: Unsupported.
BridgeFL occurs on natural wonder OceanTile: Unsupported.
BridgeFR occurs on natural wonder CoastTile: Unsupported.

Striving to clean up warnings and tighten the mod for quality improvement, and better modder etiquette, is desirable. Not being able to resolve these warnings, literally having no means, except stripping out core gameplay elements, is not a good solution. Yellow flagged on top of irremovable? seems overly squelchy.

The real problem is not on the modders' end, it's the players' end.

Why does the player need to be inundated with broken mod warnings at the Start Game screen, when the mod is not broken? I'd use terms like suffer and punishment on the player's behalf from a UI/UX perspective; such warnings add unnecessary confusion. btw that's actually a general observation not just limited to the mod in question.

The concept of suppression comes into play because it's already being utilized: G&K GlobalUniques.json

        // Filtering uniques must be listed here to tell RulesetValidator they're OK despite untyped
        "Aircraft"

Comment that line out:

Can we do two things?

1. Remove the squelchy pop-up in the Start Game screen, or turn it down with maybe a friendly summary? A modder will know (or for aspiring noobs as we've all been it can be displayed as a trailing info line) to go to modchecker for further information.
2. Add either a static suppressor like "Aircraft" above, or a dynamic suppressor for unresolvable but unbroken MC complaints. I doubt the latter will pass qc tbh.

[SomeTroglodyte]

G&K GlobalUniques.json

Thanks for the catch because that is outdated with 4.8.16 and should be removed. The message you show should no longer show, and if it does, has different text.

-> #10435

[hackedpassword]

Aircraft message is suppressed.

Unsupported complaints remain. UI complaints in New Game screen should not be scaring the player into thinking their device will melt down if they hit that red button.

3242

[SomeTroglodyte]

Notes - unordered, no specific validity filters:

• I believe we should explore the idea - I've been carrying it around for months. Issue title perfect ~- OK a little more specific...~
• Storage - ModOptions close to inter-mod compatibility (which should some day also progress)? Would mean another meta UniqueType, or - new field.
• Crass: Use PKI so we must sign each individual suppression request
• Lax: One RegEx operating on the literal warning text, or an array of RegEx'es -> If RegEx then storage as Unique is not good, our placeholder parsing has no escaping. Or simple array of literal warning text to suppress...
• Is there any middle way possible that's both flexible and not trivial to abuse? No idea.
• Suppressed should still display in ModChecker, maybe new color - Blue
• Side note: My idea to store mod check results and ask users to explicitly accept on NewGameScreen is broken. The implementation that is, not the idea: The result easily gets out of sync with the actually active mods. Not fixed yet as the entire NewGameScreen is a sore spot in my eyes and both layout and architecture need changes - can't touch much without accidentally deleting and redesigning from scratch. (I'm envisioning - as disgusting as they are - wizard-style. Pages, left to right, right respecting and limiting to decisions on farther left pages, left pages absolutely free from interdependencies with further right pages. Entire pages can appear/disappear depending on choices to their left. Would make clean implementation sooooo much easier)

[SomeTroglodyte]

An inventory, focus "kinds of usecases", random mod selection:

• "Mod has no configuration for tileset graphics": The test is geared for actual TileSet mods, but e.g. "3rd and 4th Unique Component" just adds extra graphics. That usecase may also wish to add, not replace ruleVariants. No idea whether a merge is coded - if it is, can the test stay as is and it's better the modder just adds an empty config? If not, the usecase need a solution.
• "Fallback tileset invalid: Minimal" - that's a hardcoded value not present as folder, which is what the test sees. But I believe TileSet mods shouldn't use it directly, fallback should be mod-builtin-minimal? Don't know. Needs solution.
• "Clan Horseman obsoletes at tech Rifling, and therefore Redomestication for its upgrade Hussar may not yet be researched!" - might be a mistake in the texh tree or a classic case for "this is intentional, shut up"...
• "colors do not contrast enough" might be another example why a suppression method is really needed.
• The "not typed, not filtering, but I want it displayed just as informational message" usecase - is it valid because such messages need to be displayed outside Civilopedia (in which case I'd do a UniqueType.Comment) or is the option civilopediaText the correct answer? Is 'British East India Company's unique "Cheaper to buy"' such an example? @Caballero-Arepa - were you the '3rd and 4th Unique Component' maintainer? If so, elucidate? Ooooh - a non-pedia "comment" type may synergize with the new hidden modifier to replace ugly working, typed uniques with a "nicer sounding" one, e.g. because 4 Uniques are needed to allow a list of resources - ?? Other intended comments may have another correct answer - more Civilopedia autolinking code. I see "Unlocked at Education" here - no effect - because it's implemented on the tech, but not automatically shown on the Improvement? Could be an example.
• I see a lot of 'really untyped, sounds linke intended effect, but does nothing because untyped' stuff where I think an actual implemented alternative exists - good.
• I see misspellings including trailing spaces ruining the UniqueType match - remember the typographical quotes - maybe consider adding intelligence to the "spellcheck" path, and add a "spellcheck" kind of helper to the parameters too?

[Caballero-Arepa]

@Caballero-Arepa - were you the '3rd and 4th Unique Component' maintainer? If so, elucidate?

Ehm, no, I have collaborated, but maintainer is @EmperorPinguin .

But: I agree a 'comment' unique would be useful, as these are placed and used to inform the user of something or simplify a series of uniques. For example that "Cheaper to Buy", you dont place that in the Civilopedia because you don't want the user to go to the Pedia to know critical information/benefits about the construction.

So yeah, you are right about that.

We shouldn't display civilopedia text in the city screen or new game screen because it's precisely for civilopedia.

[SomeTroglodyte]

Ehm, no, I have collaborated, but

Wild guess from that Llanero.png in there, was too lazy to actually check 😇 "Y ahora, en vivo en nuestro Mod, Calores Vivantes con sus Canciones de la Provincia!"

[SeventhM]

Clan Horseman obsoletes at tech Rifling, and therefore Redomestication for its upgrade Hussar may not yet be researched

I was looking at this at one point and this one is actually interesting. Still a case of "shut up, this is intentional", but actually validator is at fault. Its upgrade is on the same tech as it, which isn't counted correctly since Redomestication is not in the tech path of Rifling

[SomeTroglodyte]

on the same tech as it

See - actionable - I'll need to start collating a checklist. But soooo tired. Whine. :cat2: :zzz:

• [ ] Implement Comment Unique (key trick: Typed, key text with type "comment" parameter, key text disappearing via translation engine)
• [ ] Implement Suppression - key decision: Unique blocking Regex-capable future upgrade or new array field, (team signing requirement - nah)
• [ ] Fix "obsoletes at tech" validation when same tech
• [ ] See about spellcheck part of RulesetValidator and mistakes type trailing space / punctuation, can it be made cleverer to find the intended UniqueType
• [ ] See about spellcheck-type intelligence for parameter validation
• [ ] Check extension mod merge code for TileSet ruleVariants merge
• [ ] Check "Minimal" fallback meaning for pure "new" TileSet mods
• [ ] Check requirements for "mixed" ruleset extension with a few TileSet extras - what should we actually check and is there a pattern when to relax validation
• [ ] NewGameScreen "accept" code fix

Now who wants to take over coding what?

[Caballero-Arepa]

Ehm, no, I have collaborated, but

Wild guess from that Llanero.png in there, was too lazy to actually check 😇 "Y ahora, en vivo en nuestro Mod, Calores Vivantes con sus Canciones de la Provincia!"

What can I say 😏 - "¿Me lleva él, o me lo llevo yo?"

That song... 👌🤌

[SomeTroglodyte]

Other "Cultures" have ~fakes~ heroes like this one as pinnacle of local popular music...

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 15 days.

[SeventhM]

We have comment uniques and #11142. Is this still relevant?

[Caballero-Arepa]

I still think that

turn it down with maybe a friendly summary?

is needed. You often see users complaining about some yellow errors, who don't understand what these mean and think it's buggy.

[SeventhM]

Sure, I guess. I'm not against making the warning a bit friendlier, though that's not my expertise. Main concern there is kinda suspect any warning there will set off user complaints, and the warning still is necessary (that's why we have two separate categories of yellow warnings, on the player sees and one they don't

Probably the main thing is maybe the user facing warning shouldn't also include settings only warnings. That would at least maybe reduce the list down for some mods that have green "warnings" for filtering uniques

[Caballero-Arepa]

Yeah, a common player shouldnn't get a list of all the wrong uniques in the face. Instead, they should get something like "These mods have minor/severe/possible mistakes. You can play it, but don't expect everything to work. Contact the mod creator to solve these issues" And then a list of the mods that have these errors.

[hackedpassword]

Contact the mod creator to solve these issues

Mmm I still think the above is a bad deference. Isn't that the same headache that yairm210 endures (at a larger scale)? Hmm.

Over in AF, the concept works. However the mod remains active, and is a top mod. What if the mod is not being maintained or abandoned? That becomes player frustration.

How about Information like:

"This mod was auto-checked revealing potential glitches. These are not harmful, but issues found may have the following effects:

• Implement gameplay features not currently supported, though may work fine
• Create a buggy game experience
• Worst case, crash the game - please submit a report if so!

The mods' github repo is located at [repo_url]. Mod issues may be reported there, game issues can be reported to [Unciv_repo], and community discussions are held in [discord]. PSA: Try modding yourself! See [modder_info_section]

Mod issue info: :> [drop down arrow] click for issue details ... [mod checker info] "

Language above is posed as non-alarming, player-focused. :1st_place_medal:

[Caballero-Arepa]

Definitely that's much much better.

[hackedpassword]

Improved idea:

It could be practical to implement a manual suppression mechanism that allows the modder to control known issues in a way that's not lost in analysis, friendly, easily applied:

"turnsInto": "!Force", // Suppressed because the modder acknowledges the unresolvable modchecker conflict
"uniques": [
            "!Label", // Suppressed being an intended label
            "!Double movement in [Plains] <when not at war>", // Suppressed as this combo may work as desired acknowledging dev expectations
], 

Modchecker output

Modchecker complaint A Modchecker complaint B ...

Suppressed messages: 3

Modchecker complaint 1 Modchecker complaint 2 Modchecker complaint 3

On the New Game screen, the drop down would not be present, only the counted summary.

[hackedpassword]

Suppression is built in. #11142 🥳

https://yairm210.github.io/Unciv/Modders/uniques/#triggerable-uniques

My gratuitous appreciation for implementing this @SomeTroglodyte

[SomeTroglodyte]

gratuitous

"Unearned, Unjustified" :rofl: :zany_face: :joy_cat: