Skip to content

coils: pass a lamp coil's strength to the lamp instead of on/off - #586

Draft
dcbenji wants to merge 1 commit into
freezy:masterfrom
dcbenji:feat/physical-outputs-lamp-coils
Draft

dcbenji wants to merge 1 commit into
freezy:masterfrom
dcbenji:feat/physical-outputs-lamp-coils

Conversation

@dcbenji

@dcbenji dcbenji commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

AI disclosure: Ben was working on flasher domes for an Attack from Mars conversion in VPE and wondered why VPE doesn't get the pulse-modulated (PWM) incandescent fading that VPX has. While investigating, we (Ben + Claude) found that even when the gamelogic engine sends a flasher's brightness, CoilPlayer reduces lamp-mapped coils to on/off. This PR was written by Claude (AI) at Ben's request; Ben will review it before it leaves draft. Please review it with that in mind.

What VPX does

VPX tables with PWM flashers (UseVPMModSol = 2, or modulated solenoids) get a 0-1 level per flasher in SolModCallback and set it as the light's state, e.g. Flasher.State = value. A #89 flasher bulb then ramps up in ~20 ms and glows down over ~200 ms, following the ROM's actual pulses and PWM.

What VPE was missing

CoilEventArgs already carries a normalized strength (Value, 0-1), and device coils get it (CoilPlayer.cs:186, coil.OnCoil(coilEvent.Value)). Coils mapped to a lamp don't:

  • CoilPlayer.cs:164 calls _lampPlayer.HandleCoilEvent(coilEvent.Id, coilEvent.IsEnabled).
  • LampPlayer.cs:198-201 turns that into LampStatus.On/Off.

So a flasher whose GLE sends its brightness (the PinMAME PR below does once physical solenoids are on) is fully on while the bulb glows at any level, and then snaps off.

Change

  • New LampPlayer.HandleCoilEvent(string id, float value). For each lamp mapped to the coil, a value above 0 sets the lamp On and then its intensity to the value, and 0 sets it Off. Status comes first because an insert's On status means full intensity, and a light shows its intensity only while it is on. Off keeps the last level, as a status Off does. RGB mappings stay on/off.
  • CoilPlayer passes coilEvent.Value to it.
  • The bool overload is unchanged.

Plain coils send 0 and 1 (CoilEventArgs(id, bool) sets Value to 0/1), so for every existing table the result is the same as before: On at full intensity, or Off. Only GLEs that send fractional coil values see a difference.

GI levels need no Engine change. The PinMAME PR scales physical 0-255 GI into the mapping's FadingSteps (8 by default), so existing GI mappings keep working.

How tested

  • New EditMode tests LampCoilValueTests: a fractional strength drives the light's intensity, the sequence 0.3 → 1 → 0.05 → 0 is followed, and a plain coil (1/0) gives On at full intensity / Off. They compile but have not been run. No Unity editor was available for this (another session had it), so please run them with the EditMode suite.
  • The VisualPinball.Unity runtime assembly of this branch builds outside Unity against Unity 6000.5.0f1's managed DLLs (dotnet build, 0 errors, same as master; Collections/CollectionExtensions.cs excluded on both because it needs Unity's internals access).
  • VisualPinball.Engine.Test (dotnet test on Linux, rolled forward to .NET 8): 108 passed / 22 failed, identical on master. The failures are pre-existing mesh-fixture differences on Linux, e.g. ShouldGenerateACube.
  • What the levels look like: with the PinMAME PR, AFM's flasher 17 (Tests: Switch from xUnit to NUnit #89) reports 9 → 156 → 244 → 255 over ~50 ms, then 200, 117, 69, 43, 27, 17, 11, 8, 5, 3, 2, 1, 0 over ~240 ms (headless libpinmame 3.7.0). Before this change the light would be on for that whole sequence and then off. The AFM conversion worked around this with a table-side script: it moved the flasher coils out of the lamp mappings and drove those lights itself, using a Unity-side Tests: Switch from xUnit to NUnit #89 approximation fed by the coils' on/off timing. This PR makes that workaround unnecessary.

Notes

  • The light's own FadeSpeedUp/Down still applies on top. The PinMAME PR turns those fades off for lights it drives with physical outputs, to avoid fading twice.
  • For LightComponent a coil event can now mean two FadeTo calls (status, then intensity) in the same frame. The second one wins.

Dependencies

🤖 Generated with Claude Code

Coils mapped to a lamp (flashers) went through
LampPlayer.HandleCoilEvent(id, bool), which only sends LampStatus.On or
Off, so CoilEventArgs.Value was dropped. Device coils already get the
value (e.g. PWM-modulated magnets). With PWM-integrated flashers, such as
PinMAME's physical outputs where a freezy#89 flasher bulb ramps up over ~20 ms
and glows down over ~200 ms, the bulb's brightness never reached the
light.

Add LampPlayer.HandleCoilEvent(id, float) and use it from CoilPlayer. For
a value above 0 it sets the lamp On and then its intensity to the value,
0 sets it Off. Plain coils send 0 and 1, which ends up exactly as before
(On at full intensity, Off). RGB mappings keep on/off. The bool overload
is unchanged.

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

greptile-apps Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium impact] Changes how coil strength drives lamp brightness.

The PR appears safe to merge, with a non-blocking request to test brightness forwarding through CoilPlayer.

Findings

  1. P2 Tests skip brightness forwarding ▶

Summary

This PR lets lamp-mapped coils carry fractional brightness instead of only on/off.

  • Lamp-mapped coils now set lamp brightness from their strength.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A["OnCoilChanged"] --> B["CoilPlayer"]
  B -->|"Value"| C["LampPlayer.HandleCoilEvent"]
  C --> D{"RGB mapping?"}
  D -->|"Yes"| E["Apply on/off status"]
  D -->|"No"| F["Clamp strength to 0–1"]
  F --> G["Apply status"]
  G --> H["Apply intensity when positive"]
Loading

Reviews (1) · Last reviewed commit: "coils: pass a lamp coil's strength to th..." · Reviewed by Greptile

using var fixture = new Fixture();

// a PWM-integrated flasher (e.g. PinMAME's physical outputs) reports its brightness
fixture.LampPlayer.HandleCoilEvent("17", 0.5f);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Tests skip brightness forwarding

All three tests call LampPlayer.HandleCoilEvent directly. None checks that CoilPlayer forwards Value instead of IsEnabled. Reverting that call would leave these tests passing while fractional brightness is lost during play.

Add a test that raises OnCoilChanged with a fractional CoilEventArgs.Value through a started CoilPlayer and checks the lamp's intensity.

This branch has not been deployed

No deployments
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.

1 participant