test(network): add a regression test for #1079 (EntityRef.copy on a client) - #5384
test(network): add a regression test for #1079 (EntityRef.copy on a client)#5384soloturn wants to merge 1 commit into
Conversation
…lient) #1079 reported that copying a networked entity on a remote client and then modifying a component on the copy also modified the original, attributed at the time to the copy's EntityRef "getting directed to the last registered thing with the given net id". That mechanism no longer exists: NetworkEntitySystem#onAddNetworkComponent, which registers a NetworkComponent-bearing entity into the netId lookup, is gated @NetFilterEvent(netFilter = RegisterMode.AUTHORITY) - a client never runs it, including for a locally-created copy that happens to carry a (copied) NetworkComponent. Confirmed by running this test against a real client context via the MTE integration environment: component mutation on the copy is properly isolated from the original. No production code change - the bug doesn't reproduce. Adding the test as a permanent regression guard and documentation of why. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an integration regression test for copying a networked entity on a client. The test verifies that changing a component on the copy does not change the original entity. ChangesClient entity copy isolation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This is a localized regression-test-only change with no production behavior change, and the reported integration test passes. No actionable merge-blocking risk remains beyond normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Investigated #1079 ("EntityManager::copy works incorrectly on a remote client" - copying a networked entity and modifying the copy also modified the original, attributed at the time to the copy's
EntityRefgetting redirected via net-id lookup).Traced the mechanism the original report described:
NetworkEntitySystem#onAddNetworkComponentis what registers aNetworkComponent-bearing entity into the netId→entity lookup (NetworkSystemImpl#netIdToEntityId). It's gated@NetFilterEvent(netFilter = RegisterMode.AUTHORITY)- a client never runs it, including for a locally-created copy that happens to carry a (copied)NetworkComponent. That's the mechanism the 2014 report needed to exist for the redirection to happen, and it doesn't apply client-side.Confirmed by running an actual test against a real client context via the MTE integration environment (
ModuleTestingHelper#createClient()): creating aNetworkComponent-bearing entity on a client, copying it, and mutating a component on the copy leaves the original untouched.No production code change - the bug doesn't reproduce against current
develop. Adding the test as a permanent regression guard and documentation of why, since the entity system has been substantially rewritten since 2014 and nothing currently links back to this issue.Test plan
EntityCopyOnClientTest(1/1 pass) via:engine-tests:integrationTest.