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

Avoid mixing up truckfiles when filename isn't unique. #3171

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ohlidalp
Copy link
Member

@ohlidalp ohlidalp commented Aug 1, 2024

Fixes #3166

Problem1: When spawning an actor, the cache entry was always looked up via truckfile name, even if it was already known from the Selector UI. This potentially caused mixups.

Fix2: Use the entry provided by Selector UI.

Problem2: in many scenarios, the game selects an actor by truckfile name alone:

  • savegames
  • preselected truck in RoR.cfg
  • preloaded trucks on terrain (.tobj)
  • Multiplayer spawn notifications

Fix2: introduce an optional "Bundle-qualified" syntax, i.e. "mybundle.zip:myactor.truck" (in case of ZIP archive) or "mysubdir:myproject.truck" (in case of subdir) and use it in all the above cases. Because it's optional, it doesn't need a version bump in savegame format or RoRnet protocol. Note that 'bundle' is RoR jargon for a ZIP archive or directory under /mods.

Testing

Use the attached mods and spawn them using console commands:
dafsemiMixupTest_hangar.zip
dafsemiMixupTest_transred.zip

This uses just filename - it will spawn the first one in your 'mods.cache' JSON file.

as game.pushMessage(MSG_SIM_SPAWN_ACTOR_REQUESTED, { {'filename', 'dafsemiMixupTest.truck'}, {'position', game.getPersonPosition()}, {'rotation', quaternion(game.getPersonRotation(), vector3(0,1,0))} });

This uses explicit bundle name, so it will spawn the Transred variant.

as game.pushMessage(MSG_SIM_SPAWN_ACTOR_REQUESTED, { {'filename', 'dafsemiMixupTest_transred.zip:dafsemiMixupTest.truck'}, {'position', game.getPersonPosition()}, {'rotation', quaternion(game.getPersonRotation(), vector3(0,1,0))} });

This uses explicit bundle name and demonstrates case-insensitive lookup - it will spawn the Hangar variant.

as game.pushMessage(MSG_SIM_SPAWN_ACTOR_REQUESTED, { {'filename', 'DAFSEMIMIXUPTEST_HANGAR.ZIP:DAFSEMIMIXUPTEST.TRUCK'}, {'position', game.getPersonPosition()}, {'rotation', quaternion(game.getPersonRotation(), vector3(0,1,0))} });

Copy link
Collaborator

@CuriousMike56 CuriousMike56 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate files spawn OK now, however trying to load a savegame with them always crashes.

@ohlidalp
Copy link
Member Author

ohlidalp commented Aug 3, 2024

Indeed, the savegames only record the filename. Same goes for:

  • preselected truck in RoR.cfg
  • preloaded trucks on terrain (.tobj)
  • AFAIK also RoRnet spawn notifications

I'll try the following solution: in any text field which expects filename, user can also put "mybundle.zip:myactor.truck". On Windows, : is already forbidden in filenames so this clearly indicates a special behavior - Linux doesn't have this rule but it probably should, so let's assume nobody puts colons in mod filenames.

@CuriousMike56
Copy link
Collaborator

CuriousMike56 commented Aug 5, 2024

Works nicely, though it's probably not a good idea for bundle names to be announced to other players:
2024-08-05_19-10-22
Just another opportunity for someone to name the zip/subdir something vile.

@ohlidalp
Copy link
Member Author

ohlidalp commented Aug 7, 2024

Very good point, I'll strip the names from the log messages.

EDIT: done.

Fixes RigsOfRods#3166

Problem: When spawning an actor, the cache entry was always looked up via truckfile name, even if it was already known from the Selector UI. This potentially caused mixups.

Fix: Use the entry provided by Selector UI.
…r.truck"

Problem: in many scenarios, the game select an actor by truckfile name alone:
 *   savegames
 *   preselected truck in RoR.cfg
 *   preloaded trucks on terrain (.tobj)
 *   Multiplayer spawn notifications

Solution: introduce an optional "Bundle-qualified" syntax, i.e. "mybundle.zip:myactor.truck" (in case of ZIP archive) or "mysubdir:myproject.truck" (in case of subdir) and use it in all the above cases. Because it's optional, it doesn't need a version bump in savegame format or RoRnet protocol. Note that 'bundle' is RoR jargon for a ZIP archive or directory under /mods.

Codechanges:
* CacheSystem: completely eliminated helpers `CheckResourceLoaded()` which searched across all resource groups, potentially introducing mixups. Extended `FindEntryByFilename()` to support the new Bundle-qualified syntax.
* TerrainObjectManager: don't check preselected trucks immediatelly, it will be done on spawn anyway.
* Savegame: always use the Bundle-qualified syntax.
* Actor: in multiplayer, always use the Bundle-qualified syntax when announcing spawn.
@ohlidalp ohlidalp force-pushed the 3166_Mike_truckfile_name_collision_spawn branch from 17c7abd to bb2cbbb Compare September 1, 2024 14:02
@Zentro
Copy link
Member

Zentro commented Sep 1, 2024

Works as intended.

ohlidalp added a commit to ohlidalp/RoRServerBot that referenced this pull request Sep 15, 2024
This is a bundle-qualified format, where 'bundle' is ZIP/subdir in modcache. See RigsOfRods/rigs-of-rods#3171

Code changes:
- RoR_client: new function `getTruckFilenameFromStreamName()` with commentary. Existing `getTruckInfo()` extended to use it.
- services_start: just renamed the banlist-scanning function for clarity
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.

Actors with matching filenames don't spawn correctly
3 participants