Fix asymmetric mujoco-assets/ prefix matching in buildZipCandidates - #4
Open
midasbal wants to merge 1 commit into
Open
Fix asymmetric mujoco-assets/ prefix matching in buildZipCandidates#4midasbal wants to merge 1 commit into
midasbal wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
buildZipCandidatesinsrc/lib/mujoco-asset-loader.tsmatched asset pathsasymmetrically across the two asset-prefix flavors. A reference using the
hyphen prefix (
mujoco-assets/) never generated the underscore candidate(
mujoco_assets/<stem>), so an asset stored in the zip under the underscoreprefix was missed. It also emitted useless doubled-prefix candidates such as
mujoco-assets/mujoco-assets/....Why it matters
When the XML references
mujoco-assets/...and the zip storesmujoco_assets/..., the zip lookup misses. WithzipOnlyenabled this throwsAsset not found in zipand the asset fails to load; otherwise it silentlyfalls back to an individual fetch.
Fix
Derive a single prefix-agnostic stem (strip whichever known prefix is present)
and build candidates from it:
normalized,stem,mujoco-assets/<stem>,mujoco_assets/<stem>. Matching is now symmetric.Verification
mujoco-assets/tex/wood.pngref vsmujoco_assets/tex/wood.pngentry: now matches (was a miss).mujoco_assets/...ref vsmujoco-assets/...entry: still matches.Notes
Scoped to the one function; no dependencies or config changed. The repo has no
test runner, so I did not add one here. Happy to add a regression test covering
both prefix directions if you'd like to adopt one.