Skip to content

Trigger: glTF load options are silently dropped by the forwarding allowlist #1649

Description

@obiot

A Tiled Trigger that points at a glTF/GLB level loads it with every 3D option at its default, because Trigger forwards a fixed allowlist of load options and that list predates glTF support.

src/renderable/trigger.js:93:

for (const property of [
	"type",
	"container",
	"onLoaded",
	"flatten",
	"setViewportBounds",
	"to",
]) {
	if (typeof settings[property] !== "undefined") {
		this.triggerSettings[property] = settings[property];
	}
}

type and to are the trigger's own, so four load options reach level.loadcontainer, onLoaded, flatten, setViewportBounds. Against the current option set:

forwarded dropped
container, onLoaded, flatten, setViewportBounds scale, rightHanded, lights, lightIntensityScale, castGroundShadow, shadowGroundY

Every dropped one is a glTF/GLB option. The list was complete when level.load only handled Tiled maps; glTF scenes started loading through the same call later and nothing revisited it.

Why it bites

scale is the worst of them. It defaults to 1, i.e. one pixel per glTF unit, so a metre-scale export arrives a few pixels tall. That reads as a failed load rather than as a missing option, which is a long way from the actual cause.

It fails silently. An unrecognised Tiled property is simply not copied — no warning, nothing in the console. The author sees a correctly spelled castGroundShadow on the trigger object in Tiled and no effect at runtime.

The values are fine. TMXUtils already coerces Tiled property values (Number(raw) for numerics, raw === "true" for booleans), so a scale of 50 arrives as the number 50. This is purely the allowlist, not a typing problem.

Two shapes for the fix

  1. Extend the list with the six. Smallest change, but it goes stale again the next time an option is added — which is exactly how it got here.
  2. Invert it: forward everything except the trigger's own keys — event, to, type, color, fade, duration, transition, shape, shapes, width, height, id. Does not rot, at the cost of forwarding anything an author misspells straight through to level.load, which ignores unknown keys anyway.

The second is preferable on maintenance grounds, though the exclusion list needs care: it has to stay in step with what the Trigger constructor reads for itself.

async deliberately stays out of both: triggerEvent sets it at the call site, and it should not be author-controllable from a Tiled property — the trigger needs the promise to sequence its own reveal.

Notes

  • No test covers Trigger option forwarding at all; tests/trigger_level_change.spec.js covers the transition sequencing but not which options survive the hop.
  • Worth checking whether a Tiled trigger targeting a glTF level is a case anyone has actually shipped, or whether this is latent — it affects how far the fix should go.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions