Skip to content

Match tags and uppercase mod names independent of culture - #193

Open
sparr wants to merge 1 commit into
sarbian:masterfrom
sparr:fix-turkish-locale
Open

sparr wants to merge 1 commit into
sarbian:masterfrom
sparr:fix-turkish-locale

Conversation

@sparr

@sparr sparr commented Oct 5, 2026

Copy link
Copy Markdown

Currently, the mod uses CurrentCultureIgnoreCase for various tag names and searches. In Turkish (or some other language(s)), i and I have alternative other-case forms, so :first and :final are not recognized. This PR uses OrdinalIgnoreCase instead.

Currently, the mod applies ToUpper to mod names for pass filters, which turns i into İ, triggering the same failure. This PR uses ToUpperInvariant instead.

This bug is not currently visible in normal use, because KSP's HighLogic.Awake resets the locale before the MM thread inherits it. It is, however, visible if another mod resets the main thread culture after the vanilla reset but before MM runs, or builds a MM ProtoPatchBuilder or PatchExtractor in a thread with a triggering locale.

AI Disclosure

I am working on a project that depends on KSPMMCfgParser, which led me to a larger case sensitivity problem there (KSP-CKAN/KSPMMCfgParser#25). Investigating that problem revealed this one. I tasked Claude Opus 5.5 with narrowing the problem here and finding all of the case sensitive string comparison sites. It also wrote the tests in this PR and the commit message. I wrote the PR body based on my understanding of the issue and the fix.

Root node tag names and the nested :HAS[ search were compared with
CurrentCultureIgnoreCase. Under a Turkish or Azeri culture, i and I are
not a case pair, so :first and :final in any casing other than all
uppercase were not recognized. Use OrdinalIgnoreCase instead.

Pass specifier descriptors uppercased the mod name with the current
culture, turning i into U+0130. Use ToUpperInvariant, as PatchList
already does for pass names.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sarbian

sarbian commented Oct 5, 2026

Copy link
Copy Markdown
Owner

Ah, the joy of i8n. Thanks for the patch.
At first glance it looks good to me. I currently cannot test it, so hopefully someone else can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants