In-progress rework of JavaScript World APIs. by dyfios · Pull Request #114 · Five-Squared-Interactive/WebVerse-Runtime

dyfios · 2026-04-24T00:57:32Z

No description provided.

Copilot

Pull request overview

This PR adds a first-pass event system for the JavaScript World APIs (world lifecycle, entity lifecycle/property changes, and collisions) and wires emissions into runtime/world loading and entity management, with a broad NUnit test suite for the new behavior.

Changes:

Introduces core event primitives (Events, IEventEmitter, ObserverLimits) and registers Events into the JS runtime.
Adds lifecycle emissions for World (load/ready/error) and Entity (spawn/destroy + property change events).
Adds collision event bridging via a Unity MonoBehaviour and extensive JavaScript-handler unit tests.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
Assets/Runtime/StraightFour/Entity/Mesh/Scripts/MeshEntity.cs	Attaches/removes `CollisionEmitter` for mesh entities and preview clones.
Assets/Runtime/StraightFour/Entity/Base/Scripts/CollisionEmitter.cs	New Unity collision/trigger bridge intended to emit JS collision events.
Assets/Runtime/Runtime/Scripts/WebVerseRuntime.cs	Emits `World` lifecycle events during world load success/failure.
Assets/Runtime/Handlers/JavascriptHandler/Tests/WorldLifecycle.Tests.cs	New tests for `World` event registration/unregistration and lifecycle semantics.
Assets/Runtime/Handlers/JavascriptHandler/Tests/PropertyChangeEvents.Tests.cs	New tests for entity property-change event mechanics.
Assets/Runtime/Handlers/JavascriptHandler/Tests/ObserverLimits.Tests.cs	New tests for global observer limit tracking/enforcement.
Assets/Runtime/Handlers/JavascriptHandler/Tests/NetworkingEvents.Tests.cs	New tests for HTTP static events and IEventEmitter pattern for networking APIs.
Assets/Runtime/Handlers/JavascriptHandler/Tests/MultipleListeners.Tests.cs	New tests for listener ordering and multi-listener behavior.
Assets/Runtime/Handlers/JavascriptHandler/Tests/LegacyCoexistence.Tests.cs	New regression tests ensuring event system coexists with legacy callback paths.
Assets/Runtime/Handlers/JavascriptHandler/Tests/InputEvents.Tests.cs	New tests for Input static event methods.
Assets/Runtime/Handlers/JavascriptHandler/Tests/Events.Tests.cs	New tests for event constants and validation.
Assets/Runtime/Handlers/JavascriptHandler/Tests/EventEmitter.Tests.cs	New tests for default interface method behavior on `IEventEmitter`.
Assets/Runtime/Handlers/JavascriptHandler/Tests/EntityLifecycle.Tests.cs	New tests for entity spawn/destroy semantics and disposal behavior.
Assets/Runtime/Handlers/JavascriptHandler/Tests/EntityEventEmitter.Tests.cs	New tests for `BaseEntity` implementing `IEventEmitter`.
Assets/Runtime/Handlers/JavascriptHandler/Tests/CollisionEvents.Tests.cs	New tests for collision event naming and argument passing mechanics.
Assets/Runtime/Handlers/JavascriptHandler/Scripts/JavascriptHandler.cs	Exposes `Engine` getter and registers JS-facing `Events` constants object.
Assets/Runtime/Handlers/JavascriptHandler/APIs/WorldBrowserUtilities/Scripts/World.cs	Adds static `World` event system (on/off/once/Emit/dispose + debug).
Assets/Runtime/Handlers/JavascriptHandler/APIs/Networking/Scripts/WebSocket.cs	Implements `IEventEmitter` on `WebSocket` and adds event disposal.
Assets/Runtime/Handlers/JavascriptHandler/APIs/Networking/Scripts/MQTTClient.cs	Implements `IEventEmitter` on `MQTTClient` and adds event disposal.
Assets/Runtime/Handlers/JavascriptHandler/APIs/Networking/Scripts/HTTPNetworking.cs	Adds static HTTP event system (on/off/once/Emit/dispose).
Assets/Runtime/Handlers/JavascriptHandler/APIs/Input/Scripts/Input.cs	Adds static Input event system (on/off/once/Emit/dispose).
Assets/Runtime/Handlers/JavascriptHandler/APIs/Entity/Scripts/EntityAPIHelper.cs	Emits entity `spawn` upon mapping registration.
Assets/Runtime/Handlers/JavascriptHandler/APIs/Entity/Scripts/BaseEntity.cs	Implements `IEventEmitter`, emits property-change/destroy events, adds delete guard.
Assets/Runtime/Handlers/JavascriptHandler/APIs/Core/Scripts/IEventEmitter.cs	New default-method event emitter interface + global observer limits.
Assets/Runtime/Handlers/JavascriptHandler/APIs/Core/Scripts/Events.cs	New canonical event name constants + validation registry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T01:05:43Z

+            if (result && Listeners.ContainsKey(Core.Events.Entity.Scale))
+            {
+                ((Core.IEventEmitter)this).Emit(Core.Events.Entity.Scale);
+            }


Same lazy-init/allocation issue: this Listeners.ContainsKey(...) call will allocate the listeners dictionary for entities that never use events. Consider checking the backing field (or a helper) before emitting scale events.

Copilot · 2026-04-24T01:05:44Z

+            if (result && Listeners.ContainsKey(Core.Events.Entity.Visibility))
+            {
+                ((Core.IEventEmitter)this).Emit(Core.Events.Entity.Visibility);
+            }


Same lazy-init/allocation issue: emitting visibility events currently forces Listeners initialization even when unused. Prefer a non-allocating listener check before emitting.

Copilot · 2026-04-24T01:05:44Z

+        /// <summary>
+        /// Guard flag to prevent re-entrant Delete() calls from destroy listeners.
+        /// </summary>


The XML doc comments are malformed here: a second /// <summary> block starts inside the existing Delete method documentation. This will produce invalid XML docs / warnings; merge the guard-flag description into the existing summary/remarks instead of starting a new <summary>.

Copilot · 2026-04-24T01:05:44Z

+            // Emit destroy event — listeners can still access entity properties
+            ((Core.IEventEmitter)this).Emit(Core.Events.Entity.Destroy);
+
+            // Clean up all event listeners and mark as disposed
+            DisposeEvents();
+
+            // Deregister from entity mapping (fixes pre-existing leak where
+            // RemoveEntityMapping was never called during entity destruction)
+            EntityAPIHelper.RemoveEntityMapping(internalEntity);
+
            return internalEntity.Delete(synchronizeChange);


_isDeleting is set to true but never reset. If internalEntity.Delete(...) returns false (or throws), future Delete() calls will always be skipped. Reset the flag in a finally (or only set it once deletion is guaranteed) so the entity can recover from failure paths.

Suggested change

// Emit destroy event — listeners can still access entity properties

((Core.IEventEmitter)this).Emit(Core.Events.Entity.Destroy);

// Clean up all event listeners and mark as disposed

DisposeEvents();

// Deregister from entity mapping (fixes pre-existing leak where

// RemoveEntityMapping was never called during entity destruction)

EntityAPIHelper.RemoveEntityMapping(internalEntity);

return internalEntity.Delete(synchronizeChange);

bool deleted = false;

try

{

// Emit destroy event — listeners can still access entity properties

((Core.IEventEmitter)this).Emit(Core.Events.Entity.Destroy);

// Clean up all event listeners and mark as disposed

DisposeEvents();

// Deregister from entity mapping (fixes pre-existing leak where

// RemoveEntityMapping was never called during entity destruction)

EntityAPIHelper.RemoveEntityMapping(internalEntity);

deleted = internalEntity.Delete(synchronizeChange);

return deleted;

}

finally

{

if (deleted == false)

{

_isDeleting = false;

}

}

Copilot · 2026-04-24T01:05:45Z

+        public static int CurrentCount { get; internal set; } = 0;
+


ObserverLimits.CurrentCount has an internal set, but the new JavaScript handler tests set this value directly from a separate test assembly. Without InternalsVisibleTo("FiveSQD.WebVerse.Handlers.Javascript.Tests"), those tests won’t compile; consider adding the attribute in the runtime assembly rather than making this setter public.

Copilot · 2026-04-24T01:05:46Z

+            bool result = internalEntity.SetEulerRotation(new UnityEngine.Vector3(eulerRotation.x, eulerRotation.y, eulerRotation.z),
                local, synchronizeChange);
+
+            if (result && Listeners.ContainsKey(Core.Events.Entity.Rotation))


Same lazy-init/allocation issue: Listeners.ContainsKey(...) will allocate the Listeners dictionary even when no listeners exist. Prefer a non-allocating check before emitting rotation events.

Suggested change

if (result && Listeners.ContainsKey(Core.Events.Entity.Rotation))

if (result)

Copilot · 2026-04-24T01:05:47Z

+            // Clean up all event listeners and mark as disposed
+            DisposeEvents();
+
+            // Deregister from entity mapping (fixes pre-existing leak where
+            // RemoveEntityMapping was never called during entity destruction)


This method disposes the event system and removes the entity mapping before confirming that internalEntity.Delete(...) succeeded. If deletion fails, JS-side references will be orphaned (disposed + unmapped) while the internal entity may still exist. Consider only disposing/removing the mapping after a successful delete (or roll back on failure).

Copilot · 2026-04-24T01:05:47Z

+        /// <summary>
+        /// Reset the counter (for testing or world unload).
+        /// </summary>
+        internal static void Reset() { CurrentCount = 0; }
+    }


ObserverLimits.Reset() is internal, but the new JavaScript handler tests call it from a separate test assembly. Add InternalsVisibleTo("FiveSQD.WebVerse.Handlers.Javascript.Tests") (preferred) or provide a public test hook so the tests compile.

Copilot · 2026-04-24T01:05:47Z

+            // Performance guard: only proceed if entity has listeners for this collision event
+            // This avoids Jint overhead for entities without collision handlers
+            if (!ownerPublic.Listeners.ContainsKey(eventName)) return;
+


This “performance guard” still forces ownerPublic.Listeners lazy initialization, which allocates the listeners dictionary even when the entity has no collision listeners. That undermines the goal of avoiding overhead for non-listening entities. Consider adding a non-allocating listener check API on the JS BaseEntity (or IEventEmitter) and using that here.

Copilot · 2026-04-24T01:05:47Z

+        internal static void Emit(string eventName, params Jint.Native.JsValue[] args)
+        {
+            if (string.IsNullOrEmpty(eventName) || !_listeners.ContainsKey(eventName)) return;
+            if (!_emittingEvents.Add(eventName)) return;


HTTPNetworking.Emit(...) / DisposeAllHTTPListeners() are internal, but the new tests call them from the separate JavaScript test assembly. This will not compile without InternalsVisibleTo("FiveSQD.WebVerse.Handlers.Javascript.Tests") on the runtime assembly (preferable to making these public, which would expose them to JS).

In-progress rework of JavaScript World APIs.

0452ac0

Copilot AI review requested due to automatic review settings April 24, 2026 00:57

Copilot started reviewing on behalf of dyfios April 24, 2026 00:57 View session

dyfios merged commit 83a0186 into 3.0.1 Apr 24, 2026
2 of 3 checks passed

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

In-progress rework of JavaScript World APIs.#114

In-progress rework of JavaScript World APIs.#114
dyfios merged 1 commit into
3.0.1from
jsapi-rework

dyfios commented Apr 24, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (result && Listeners.ContainsKey(Core.Events.Entity.Rotation))
	if (result)

Conversation

dyfios commented Apr 24, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants