Add native support for magnets#561
Merged
Merged
Conversation
Greptile SummaryThis PR adds native magnet and turntable support to the pinball runtime. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (5): Last reviewed commit: "magnets: document zero height range and ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Owner
Author
|
@greptile-apps please review. |
Owner
Author
Magnet and turntable follow-upMagnet timing and coil dispatch
Physical field model
Grab and hold behavior
Turntables
Editor and documentation
|
Owner
Author
|
@greptile-apps review |
Introduce the magnet component, API, native state map, VPX-compatible radial force update, editor inspector, toolbox prefab, and focused force-scaling coverage for the first magnet milestone.
Clamp grabbed balls to the magnet core in VPX-compatible mode, emit grab and release transitions, drive the ball-held switch, and make ReleaseBall suppress immediate recapture until the ball leaves the grab radius.
Route Strength and Radius setters through the physics state, keep negative strength as first-class repulsion, and add an eject helper that releases grabbed balls with a directed kick.
Add a review-first editor tool that scans VPX table scripts for cvpmMagnet patterns, creates selected MagnetComponent instances from trigger data, maps solenoid coils, and flags turntable or manual-review cases.
Add TurntableComponent with native physics state, motor and direction coils, VPX-compatible tangential force, visual rotation support, detector creation, editor inspector, and focused physics coverage.
Implement saturated inverse-square magnet force with velocity damping and a capped spring-damper hold path for physical grabs. Add focused coverage for force saturation, negative-strength repel, and non-teleporting hold behavior.
Document native magnet and turntable setup, force profiles, coil and switch items, script API usage, and VPX detection guidance. Add the page to the creators-guide mechanisms navigation. docs(magnets): keep guide authoring-focused Remove the script control section from the magnet documentation and keep the usage guidance centered on editor setup, coil mapping, and force profile selection. docs(magnets): narrow compatibility guidance Remove the Terminator 2 and broad VPX script wording from the VPX Compatible profile guidance, keeping the section focused on imported magnet behavior.
The VPX-compatible profile computed ratio = dist/Size, but core.vbs uses dist/(1.5*Size), which made imported magnets about twice as weak over most of the influence radius. Also damp the impulse together with the old velocity, as core.vbs does, and add a parity test that encodes the core.vbs constants numerically instead of asserting against our own implementation.
Destroying a ball only removed it from InsideOfs, which frees its bit index for reuse while MagnetState.GrabbedBalls/ReleasedBalls kept the stale bit. The next ball assigned that index was treated as already grabbed (no BallGrabbed event) or regrab-suppressed, and since no release event ever fired, MagnetApi's ball_held switch stayed closed for the rest of the session. Clear the bits and enqueue the release event before the bit index is freed; both unregister paths run under the physics lock in external-timing mode, so the queue write is safe.
Both components advertise their coils as wire destinations, but the APIs did not implement IApiWireDeviceDest, so Player never registered them with the wire player and wires targeting magnet_coil, motor_coil or direction_coil silently did nothing (e.g. a MagnaSave button wired straight to a magnet). Forward Wire() to the coil lookup like flippers and kickers do.
The tangential force only checked planar distance, so balls on ramps or wireforms crossing above the disc got curved sideways. VPX avoided this implicitly because cvpmTurntable membership came from a z-bounded trigger. Add the same height window the magnet already has, packed and editable in the inspector.
RotationAngle advanced by Speed revolutions per second, so a VPX-typical speed of 90 spun the visual disc at 5400 RPM. Speed is a force scale, not a rotation rate; map it to degrees per second through an authorable VisualSpeedFactor (default: 60 RPM at speed 90).
PackReferences returned an empty payload, so the RotationTarget binding was silently lost on every package save/reload and the disc stopped rotating. Round-trip it like the teleporter does with its kickers; plain Transform references are now registered in PackagedRefs so any component can pack them.
TurntableApi had no name/component dictionary in TableApi, so registration was silently dropped and turntables were the only device unreachable by name. Add the dictionaries and Turntable() accessors, matching the magnet.
CreateState converted transform.localPosition with the static world-to-VPX matrix, which is only correct while every ancestor up to the playfield sits at identity. Re-parenting the magnet (e.g. under its trigger or a moved group) silently shifted the force field away from the world-space gizmo. Convert the world position relative to the playfield transform instead; turntables share the helper.
The InitMagnet/InitTurnTable regexes only matched numeric literals, so scripts passing a named constant (GOTG: 'kOrbMagnetPower', Ramones: 'discspeed') were silently dropped from the scan. Match identifiers too and create the device with a default plus a manual-review note. Also align the turntable ramp defaults with cvpmTurntable (SpinUp 10, SpinDown 4 units/sec) — the previous 600/600 spun discs up 60-150x faster than VPX.
Hoist the per-ball math.pow out of the 1 kHz ball loop, skip the 64-bit release scan and membership lookup for idle magnets, and skip grab bookkeeping (hashmap lookups, bit index allocation) for plain attraction magnets that can never grab. Replace the BallEventArgs duplicate with the existing HitEventArgs and use Unity.Mathematics instead of Mathf.
PhysicsEngine.cs does not import VisualPinball.Engine.Game, so the magnet release event id added for destroyed balls needs the qualified name to compile.
Keep the optional rotation target's authored local rotation and apply the simulated turntable spin as a relative delta.
Marshal bool fields in magnet and turntable physics state structs so Burst direct calls can pass them by reference.
Add a dedicated magnet icon and wire it up in the toolbox and icon registry, replacing the reused coil icon. Assign the icon to the MagnetComponent script. Also add new physics-thread, shortcut and tilt-bob icons, and a HelpURL to the tilt bob component.
Update the live magnet physics state from play-mode inspector edits and render the height range gizmo as a cylinder.
Collider components resolve their item id from the main component's GameObject while magnets and turntables use their own, so two kinematic components on one GameObject silently overwrote each other in the transform-callback map — the losing component (e.g. a kicker keeping its capture center current) stopped receiving OnTransformationChanged. Warn at registration and name both components. Also make the startup log honest again: the kinematic count now includes colliderless transforms, so report kinematic colliders and kinematic transforms separately.
The kinematic-octree dirty flag was set for every changed kinematic transform, so a continuously moving colliderless magnet or turntable forced RebuildOctree — swept AABBs for every kinematic collider on the table — each tick batch despite no collider having moved. Only mark the octree dirty when the item actually owns kinematic colliders; before colliderless items entered this path, the flag was justified by construction.
The kinematic refresh tracks the transform's height, but grab carry is planar-only — a magnet on a lifting mech raised its height window past the held ball's z and silently released it mid-carry. A grabbed ball now stays held regardless of the range check; only the grab logic itself may release it. Adds a PhysicsState test harness so tests can drive the real update/state wiring, and documents that the carry is planar.
Eject() overwrote the ball velocity absolutely, erasing the kinematic carrier velocity the grab path maintains — firing from a moving magnet (the T2 cannon case) behaved as if the magnet were stationary, unlike releasing the coil mid-move. Add the magnet's kinematic velocity to the eject impulse; zero for stationary magnets.
Kinematic registration and transform seeding happen once at startup, so toggling Is Kinematic in play mode flipped only the state flag: enabling was a silent no-op (never seeded or scanned) and disabling left the scan running against a frozen field. Disable the checkbox during play instead of pretending it works.
The inspector-sync fix covered only magnets, so play-mode edits of turntable fields silently kept the boot-time values while the magnet next to it responded immediately. Mirror the sync: rebuild the state from the authored fields on OnValidate, preserving the runtime-owned motor, direction, speed and rotation angle, and recompute the target speed the way the API setters do.
SyncPhysicsState re-derived every converted field of CreateState by hand, so any new magnet parameter would import correctly but silently stop syncing from the inspector. Build the synced state through CreateState and preserve only the runtime-owned fields (coil state and grab bookkeeping), matching the turntable sync.
Re-capturing the visual baseline from a disc that was already driven baked the accumulated spin into the baseline, compounding an angular offset on every retarget and leaving the old disc frozen mid-spin. Restore the previous target to its authored pose before capturing the new one, so retargeting round-trips cleanly.
The magnet's kinematic velocity duplicated the step-substitution core of GetKinematicSurfaceVelocity token for token — load-bearing anti-swallow logic that would silently drift apart when tuned. Extract an item-level GetKinematicVelocityAt on PhysicsState and let both the collider surface path and the magnet field path use it.
Disabled kinematic magnets paid two hashmap lookups and velocity math per tick for a result the early-out discarded; refresh the pose after the enabled check instead. Replace the three test-only zero-velocity forwarding overloads with defaulted parameters so no caller can silently drop the carrier velocity, scan the hierarchy once at startup (every collidable is a kinematic transform component), and cover the kinematic gating and velocity plumbing with wiring tests — the code paths both review findings lived in.
GetValueByRef in the new inspector sync is an extension method from VisualPinball.Unity.Collections.
The PhysicsState test harness holds a NativeOctree field for the state constructor, which lives in the com.bartofzo.nativetrees assembly.
Support 3-D magnet attraction and rigid grab/carry behavior for mech-mounted ball pickups, including vertical ejects, editor/docs coverage, and focused physics tests.
The spatial magnet held its ball by freezing it (BallState.IsFrozen) and clamping its position each tick. Reusing IsFrozen — which the engine treats as visually static and collision-exempt — for a moving carried ball made the ball invisible (transform sync skips frozen balls), sink one radius (kicker-hole render hack), and let other balls phase through it, and the hold could never be overcome. Hold the ball with a strong 3-D critically-damped spring instead (the spatial analog of ApplyPhysicalHold), leaving it a live physics object: it renders and collides normally, and because the spring impulse is capped at Strength*dt, a hard enough hit knocks it loose — the ball then leaves the grab radius and is released, and the ball that hit it can be grabbed in turn. Release is now just dropping the hold (the ball keeps its live velocity), so spatial and playfield release collapse to one path, and the two grab methods unify into one. Also fixes the stale pose on kinematic eject, drops the dead per-tick vpxDamping and Profile coercion for spatial magnets, and rewrites the spatial tests for the force model (grab-without-freeze, moving-hold tracking, knock-loose release, coil-off release).
Add a float value to CoilEventArgs and IApiCoil.OnCoil(float), normalized to [0..1] — 0/1 for plain on/off coils, or the duty-cycle strength for PWM-integrated (modulated) coils. DeviceCoil routes it through an optional OnValue action; CoilPlayer dispatches the value; on/off consumers are unchanged (value > 0). The magnet consumes it to scale its strength by the duty cycle, so a ROM that PWMs a magnet coil (e.g. Iron Man) drives a proportional pull instead of full-or-nothing.
Treat a ball already owned by the magnet as affected even when a moving field carries its center outside the coarse radius or height window. This preserves ownership until the profile-specific hold decides that the ball has actually broken free. Restore the focused grab-bookkeeping lookup used by that gate. The existing GrabbedBallSurvivesMovingHeightWindow regression test now passes while spatial knock-loose release continues through UpdateGrab.
Replace the simulation-thread boolean coil contract with the normalized float already present in CoilEventArgs. DeviceCoil now delivers every PWM value, including changes that leave the boolean enabled state unchanged, while existing transition callbacks keep their previous semantics. Give physics-sensitive consumers a dedicated simulation-thread value callback and suppress the later main-thread value callback once that fast path is active. The magnet applies enabled state and scaled strength together, avoiding a one-frame full-power command, and a focused test covers repeated values within one enabled interval.
Separate the normalized electrical command, effective coil current, and applied force in MagnetState. Physical and Spatial magnets now advance current with authorable rise and fall time constants, using a stable implicit RL step and current-squared force instead of switching instantly at the next physics tick. Keep VPX Compatible response immediate, preserve runtime electrical state across inspector synchronization, serialize the new tuning fields, and expose them in the inspector and documentation. Focused tests cover one-time-constant rise, flyback decay, and unchanged VPX response.
Replace the softened point inverse-square law with a finite axisymmetric pole kernel. Its lateral force vanishes on-axis, peaks in an annulus near the authored pole radius, weakens with vertical air gap, and decays with a dipole-like far field without evaluating elliptic integrals. Apply a squared compact-support taper at Radius and Height Range so distant balls are rejected before the field math and the force reaches zero smoothly at the boundary. Physical and Spatial profiles now treat polarity as attraction magnitude for an ordinary steel ball, while signed repulsion remains available only through VPX Compatible.
Keep the geometric snap for VPX Compatible, but require Physical and Spatial magnets to be able to arrest the ball relative to the moving hold point within the remaining grab volume. Fast fly-bys stay under the attraction force until they slow enough to be retained instead of firing a grab event on radius entry. Remove the minimum hold-strength boost and cap the spring impulse with the actual current-derived field. Weak PWM commands therefore remain weak, while existing grabbed balls still release naturally when a collision carries them outside the grab radius.
Publish turntable speed and rotation angle as one float2 animation value in both single-threaded movement application and the external-timing triple buffer. TurntableComponent now renders from that main-thread value, and the public API exposes the same published pair. Remove the visual and getter reads from the live NativeParallelHashMap while the simulation thread may be writing it. Expand the float2 snapshot capacity for turntables and cover the paired movement handoff with a focused test.
Derive turntable acceleration and visual rotation timing from PhysicsConstants.DefaultStepTimeS instead of a magnet-owned update constant. The calculation still uses the VPX ten-millisecond time unit, but its ownership now matches the engine clock. This removes an unrelated subsystem dependency and prevents a future magnet cadence adjustment from silently retiming turntables.
Remove the script-scanning editor window instead of extending its partial VBScript parser. The heuristic could not preserve general statement syntax, runtime assignments, custom helpers, or the final configured magnet state without effectively becoming a language interpreter. Drop the menu entry, source metadata, and creator documentation that advertised automatic detection. Native Magnet and Turntable components remain available for explicit setup and coil mapping.
The ramp was chosen by comparing speed magnitudes, which made a driven direction reversal briefly coast on the friction ramp. cvpmTurnTable semantics are simpler: the motor ramp applies whenever the motor is on, the friction ramp when it coasts down.
A height range of zero means unlimited vertical reach, which the tooltips and manual now state. Also drop a dead clamp on the constant planar damping and mark the coil enabled flag volatile, since the simulation thread writes it while the main thread reads it.
Owner
Author
|
@greptile-apps review |
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
1 participant
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.
This PR adds native VPE support for magnetic mechanisms, including radial playfield magnets, center-grab magnets, spatial (mech-mounted) magnets that carry the ball in 3-D, runtime repel behavior, and VPX-style spinning turntables.
MagnetComponent, runtime state, physics integration, packaging, prefab, inspector, and table API access.cvpmMagnet, and a physical profile with a finite annular pole field and coil rise/decay time constants.IApiCoilgainsOnCoil(float)andISimulationThreadCoil.OnCoilSimulationThreadnow takes afloat— external implementers need to update.IKinematicTransformComponentout ofICollidableComponentso colliderless force fields can be kinematic and follow a moving transform, including held balls.TurntableComponent, physics, packaging, inspector, table API access, and visual rotation support.