Repository navigation
Fix OMSI 2 compatibility for undefined model variables - #2951
Conversation
thibautgnt
left a comment
There was a problem hiding this comment.
Thanks @dev-Younte, this is a careful piece of work: the original-engine diagnostic table with the order permutations is exactly the kind of evidence we need for model-variable rules, and the asset-free tests are great. The neutral glyph over D/R is a real bug worth fixing. I can't merge it as is, though, because it reverses two earlier fixes that were made on purpose.
Blocking
crates/omsi-sim/src/vehicle/instance.rs:91andcrates/omsi-model/src/lib.rs:213undo two deliberate fixes.- f9e3e6d made an undeclared
[visible]variable read 0. The props.rs comment this PR removes gives the reason: Omsi.exe registers every[visible]/[alphascale]variable while reading the model (0x5efae8 -> VarList_Register). That commit fixed destination displays showing every variant at once. - #2217 (12a84ed) then registered model-only names in the runtime table.
- With this PR, a model-only selector (several meshes
[visible] X 0/1/2…) shows all of its variants stacked again, the exact overlap f9e3e6d fixed. - Your observation (missing names shown at every comparison) contradicts that disassembly note, so please reconcile the two before we change the rule. A good test: in the original, does a
.cti[setvar]on a name only the model uses switch the mesh? If it does, OMSI does register the name, and "unresolved" is not the right model.
- f9e3e6d made an undeclared
- Removing the slots makes every outside write to those names a silent no-op.
VehicleInstance::set_var(vehicle/vars.rs:73) returns false for a name with no slot, so on model-only names these now do nothing: paint-scheme[setvar](at spawn ininstance.rsput, and every frame inapply_scheme_vars_each_frame),--setvar, and a plugin'somsi.setVar. Repaints that toggle model parts through[setvar]without a varlist entry would stop working. If the original really treats a never-written name differently from a declared one, keep the registered slot, mark it "model-only, not written yet", and skip its conditions only until something writes it. That covers both your table and[setvar].
Smaller
crates/omsi-sim/src/scenery/mod.rs:263still reads an undeclared[visible]variable as 0, so the same model would behave differently as a scenery object and as a vehicle. The rule we settle on should cover both, or the PR should say why not.mod model_variables;sits aftermod scripts_props;invehicle/tests/mod.rs, and thematerialclosure intests/model_variables.rsisn't rustfmt-formatted.
Test builds for 3f59d05
|
Addressed in 7bcfdb6: model-only names keep a slot once something writes them, see the approval.
thibautgnt
left a comment
There was a problem hiding this comment.
I pushed 7bcfdb6 on top of your commit to settle the two blocking points:
- A name only the model's
[visible]uses still starts unresolved, as in your OMSI 2 table (all comparisons shown,[matl_change]on its base material). The vehicle now remembers those names, and the first write gives them a slot: a.cti[setvar](at spawn and every frame),--setvaror a plugin'somsi.setVar. From then on they are compared like a declared variable, so repaints that switch model parts this way keep working. New test:a_setvar_on_a_model_only_name_switches_its_meshes(bothset_varand a paint scheme at spawn). - Scenery objects keep the 0 rule for now: openOMSI writes engine inputs such as
NightlightAthere only when the object declares them, so skipping undeclared conditions would show day and night variants together. That needs its own original-engine check. mod model_variablesis in order and the test file is rustfmt-formatted.
Checked: cargo test --workspace on this branch merged with today's main, size-budget and arch-guard. Perf on the Radeon 680M box (Spandau, EN92, traffic 30, 100 s runs, main thread CPU): PR 6.39 / 7.36 ms, main 6.90 / 7.32 ms, so no difference beyond noise.
|
Merged, thanks @dev-Younte! Your original-engine diagnostic, with mesh and varlist order swapped, is what let us settle this rule. The extra lit N over D/R on the Town is gone. I added a small change on top so a |
Summary
Match original OMSI 2 when a model references an undefined variable: ignore its
[visible]comparison and retain the base material for[matl_change]. Declared variables continue to use their stored values.This fixes an extra illuminated neutral glyph overlapping D/R without forcing the mesh to be hidden. It is independent of the illuminated-material coverage fix in #2942.
Problem
openOMSI currently creates runtime slots for names found only in model visibility conditions. If their comparisons agree, it initializes the slot to that comparison value; otherwise it uses zero. A missing name compared with 1 can consequently both show a mesh and activate a material variant elsewhere.
Root cause
The inferred slot erases the distinction between an unresolved name and a declared variable.
[visible]and[matl_change]then read an invented value from the shared runtime table. An earlier local proposal to initialize these slots uniformly to zero also disagreed with original OMSI 2: missing names compared with 1 or 2 still leave their meshes visible in the original engine.Solution
No renderer/shader changes, vehicle-specific names or original vehicle resources are included. The diff contains only five Rust source/test files:
omsi-model/src/lib.rs,omsi-sim/src/vehicle/{instance.rs,props.rs,tests/mod.rs,tests/model_variables.rs}, all undercrates/.Validation
Based on official
mainatccc795229068ae2764ac35897aaeb80a3c9882ab.Original OMSI 2 observations: the owner manually ran an original, generated diagnostic vehicle, confirmed colour and SET-button controls, and supplied photographs. Forward mesh order, reversed mesh order and reversed varlist order gave identical initial results:
[visible]comparison 0 / 1 / 2[matl_change]A private openOMSI loader/runtime probe matches 99/99 initial case observations across these three variants. The material comparison records variable-based selection; rendered colours are checked separately below. Original-engine tests were user-operated, not automated execution of the proprietary engine.
Standalone PR 2 validation, Windows x64 GNU:
automatic_N/D/Rscript triggers through two N → D → N → R cycles. Selected glyph conditions followed 1/4/0; the undefined neutral instance stayed visible with its base material.Combined-build checks: a separate PR 1 + PR 2 Debug 0.3.38 build renders clean N/D/R glyphs and white pointers/rims in both modes. D92 captures are pixel-identical to the preserved earlier combined build in both modes. The owner additionally drove this new combined build in Vanilla and Enhanced and confirmed repeated N → D → N → R → N selection, no extra N during D/R, power OFF/ON, instruments/switches, white pointers/rims and driving behaviour. This manual feedback is for the combined build, not a PR-2-only playtest.
How to test
Start with the asset-free library tests; no bus mod, original textures/models or OMSI installation is required:
Optional in-game verification
With a separately installed OMSI vehicle using missing visibility/material names, compare against the original engine. For Elec City Town PIEV, power the vehicle, apply the brake and select N → D → N → R → N repeatedly, then power OFF/ON. Check the central selected glyph for an illuminated extra N; the physical selector's fixed D/N/R artwork and blue N background are not duplicate dynamic indicators. The vehicle is optional and is neither included in this PR nor required by its automated tests or CI.
Checking the pointer/rim fix together with this change requires the separate #2942 material patch. Either PR can otherwise be built, tested and merged independently; their current changed-file sets do not overlap.
Known limitations
0xC00000FD. A fresh debugger trace stops inD3DReflectfollowed by repeatedD3DCompiler_47.dllframes on a renderer-pipeline worker. The retained unmodified-upstream control trace shows the same recursion; that control is engine-code-identical to this upstream base (the intervening update is README-only). This existing FXC/backend issue is not fixed here. Enhanced Release rendering was not certified.Checklist