Refactor IEveSpaceObjectChild interface into a base class#29
Draft
filipppavlov wants to merge 13 commits into
Draft
Refactor IEveSpaceObjectChild interface into a base class#29filipppavlov wants to merge 13 commits into
filipppavlov wants to merge 13 commits into
Conversation
The new base class has support "owner", "parent" and "part tag" shared attributes.
There was a problem hiding this comment.
Pull request overview
Refactors the space-object child API from the legacy IEveSpaceObjectChild interface into a new EveSpaceObjectChild base class, and updates many child implementations and owners to use the new type while exposing new shared attributes (owner/parent/partTag) to Blue/Python.
Changes:
- Replaces
IEveSpaceObjectChildusage in many call sites withEveSpaceObjectChild(including pointer/vector typedefs and factory/controller paths). - Propagates and maintains owner/parent/partTag state in several child-owner types (e.g., containers, plugs, refs, sockets, procedural containers).
- Updates Blue exposure to include
EveSpaceObjectChildand addsGetOwner/GetParent/partTagexposure in many child classes.
Reviewed changes
Copilot reviewed 95 out of 95 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| trinity/trinity.cpp | Switches world-transform support/type docs to EveSpaceObjectChild. |
| trinity/Eve/UI/EveEllipseSet.h | Migrates to EveSpaceObjectChild base; removes redundant child API members. |
| trinity/Eve/UI/EveEllipseSet.cpp | Removes legacy name/LOD no-op implementations. |
| trinity/Eve/UI/EveEllipseSet_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag to Blue. |
| trinity/Eve/Turret/EveTurretSet.h | Updates ambient effect child types to EveSpaceObjectChild. |
| trinity/Eve/Turret/EveTurretSet.cpp | Updates ambient effect APIs/fields to new child type. |
| trinity/Eve/Turret/EveTurretFiringFX.h | Replaces interface vector declarations with class-based ones. |
| trinity/Eve/SpaceObjectFactory/EveSOF.cpp | Updates SOF child binding/copy paths to EveSpaceObjectChild. |
| trinity/Eve/SpaceObject/Utils/fxAttributes/EveCameraFxAttributes.h | Updates include to new child base. |
| trinity/Eve/SpaceObject/EveSpaceObject2.h | Updates effect-children API and storage to EveSpaceObjectChild. |
| trinity/Eve/SpaceObject/EveSpaceObject2.cpp | Maintains owner pointers for effect children on init/list events/destruction. |
| trinity/Eve/SpaceObject/Children/SmartLightSets/EveChildSmartLightSet.h | Migrates to EveSpaceObjectChild; removes redundant name/bounds stubs. |
| trinity/Eve/SpaceObject/Children/SmartLightSets/EveChildSmartLightSet.cpp | Removes legacy name accessors. |
| trinity/Eve/SpaceObject/Children/SmartLightSets/EveChildSmartLightSet_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/SmartLightSets/attributeModifiers/EveSmartLightBaseAttributeModifier.h | Updates include to new child base. |
| trinity/Eve/SpaceObject/Children/ProceduralContainer/EveChildProceduralContainer.h | Migrates member types + adds owner/partTag propagation overrides. |
| trinity/Eve/SpaceObject/Children/ProceduralContainer/EveChildProceduralContainer.cpp | Registers/unregisters selected child; propagates owner/partTag to selected child. |
| trinity/Eve/SpaceObject/Children/ProceduralContainer/EveChildProceduralContainer_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/IEveSpaceObjectChild.h | Deprecates interface to an empty compatibility shell; updates parent type. |
| trinity/Eve/SpaceObject/Children/IEveEffectChildrenOwner.h | Updates effect-children owner API to EveSpaceObjectChild. |
| trinity/Eve/SpaceObject/Children/EveChildSpherePin.h | Updates interface comment references. |
| trinity/Eve/SpaceObject/Children/EveChildSocket.h | Migrates to base class; updates effect-children owner signatures; adds owner/partTag propagation. |
| trinity/Eve/SpaceObject/Children/EveChildSocket.cpp | Registers/unregisters plug child; updates effect-child APIs; propagates owner/partTag to plug. |
| trinity/Eve/SpaceObject/Children/EveChildSocket_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildRef.h | Migrates to base class; updates effect-children owner signatures; adds owner/partTag propagation. |
| trinity/Eve/SpaceObject/Children/EveChildRef.cpp | Registers/unregisters referenced child; loads child as EveSpaceObjectChild; propagates owner/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildRef_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildQuad.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildQuad.cpp | Removes legacy name/renderables stubs. |
| trinity/Eve/SpaceObject/Children/EveChildQuad_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildPostProcessVolume.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildPostProcessVolume.cpp | Removes legacy name/visibility/update stubs. |
| trinity/Eve/SpaceObject/Children/EveChildPostProcessVolume_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildPlug.h | Migrates object list type; updates effect-children owner signatures; adds owner/partTag propagation. |
| trinity/Eve/SpaceObject/Children/EveChildPlug.cpp | Registers/unregisters children list; updates bounds/shader iteration types; propagates owner/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildPlug_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildParticleSystem.h | Migrates to base class; removes redundant legacy name member/accessors. |
| trinity/Eve/SpaceObject/Children/EveChildParticleSystem.cpp | Removes legacy name accessors. |
| trinity/Eve/SpaceObject/Children/EveChildParticleSystem_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildParticleSphere.h | Migrates to base class; removes redundant legacy name member/accessors. |
| trinity/Eve/SpaceObject/Children/EveChildParticleSphere.cpp | Removes legacy name accessors. |
| trinity/Eve/SpaceObject/Children/EveChildParticleSphere_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildMesh.h | Migrates to base class; removes redundant legacy name member/accessors. |
| trinity/Eve/SpaceObject/Children/EveChildMesh.cpp | Removes legacy name accessors. |
| trinity/Eve/SpaceObject/Children/EveChildMesh_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag (and adds interface define). |
| trinity/Eve/SpaceObject/Children/EveChildLink.h | Updates interface comment references. |
| trinity/Eve/SpaceObject/Children/EveChildLineSet.h | Migrates to base class; removes redundant legacy name accessors. |
| trinity/Eve/SpaceObject/Children/EveChildLineSet.cpp | Removes legacy name accessors. |
| trinity/Eve/SpaceObject/Children/EveChildLineSet_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildLightingOverride.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildLightingOverride.cpp | Removes legacy name/update stubs. |
| trinity/Eve/SpaceObject/Children/EveChildLightingOverride_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildInstanceMeshRenderer.h | Updates include to new child base. |
| trinity/Eve/SpaceObject/Children/EveChildInstanceMeshRenderer_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildInstancedMeshes.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildInstancedMeshes.cpp | Removes legacy name/LOD stubs. |
| trinity/Eve/SpaceObject/Children/EveChildInstancedMeshes_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildInstanceContainer.h | Migrates to base class; updates source/instance types; adds owner/partTag propagation. |
| trinity/Eve/SpaceObject/Children/EveChildInstanceContainer.cpp | Registers/unregisters children; updates instancing paths to set owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildInstanceContainer_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildFogVolume.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildFogVolume.cpp | Removes legacy name/update/visibility stubs. |
| trinity/Eve/SpaceObject/Children/EveChildFogVolume_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildExplosion.h | Migrates child pointer/vector types to EveSpaceObjectChild. |
| trinity/Eve/SpaceObject/Children/EveChildExplosion.cpp | Updates instance-copy variables to EveSpaceObjectChildPtr. |
| trinity/Eve/SpaceObject/Children/EveChildExplosion_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildEffectPropagator.h | Removes unused shared-child member. |
| trinity/Eve/SpaceObject/Children/EveChildEffectPropagator.cpp | Registers/unregisters effect child via helper calls. |
| trinity/Eve/SpaceObject/Children/EveChildEffectPropagator_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildContainer.h | Migrates to base class; updates child list types; adds owner/partTag propagation overrides. |
| trinity/Eve/SpaceObject/Children/EveChildContainer.cpp | Registers/unregisters children list and handles list-modified updates; propagates owner/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildContainer_Blue.cpp | Updates enum registration to new origin enum type; exposes base + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildCloud2.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildCloud2.cpp | Removes legacy name/visibility/renderables stubs. |
| trinity/Eve/SpaceObject/Children/EveChildCloud2_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildCloud.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildCloud.cpp | Removes legacy name accessors. |
| trinity/Eve/SpaceObject/Children/EveChildCloud_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildBulletStorm.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildBulletStorm.cpp | Removes legacy name/setup/update stubs. |
| trinity/Eve/SpaceObject/Children/EveChildBulletStorm_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildBehaviorSystem.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildBehaviorSystem.cpp | Removes legacy name/bounds/LOD stubs. |
| trinity/Eve/SpaceObject/Children/EveChildBehaviorSystem_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/SpaceObject/Children/EveChildAudio.h | Migrates to base class; removes redundant legacy methods/members. |
| trinity/Eve/SpaceObject/Children/EveChildAudio.cpp | Moves name storage to base (BlueSharedString) and updates emitter naming. |
| trinity/Eve/SpaceObject/Children/EveChildAudio_Blue.cpp | Exposes EveSpaceObjectChild + owner/parent/partTag. |
| trinity/Eve/Renderable/Stretch/EveStretch3.h | Migrates stretch endpoints/types to EveSpaceObjectChild. |
| trinity/Eve/Renderable/Stretch/EveStretch3.cpp | Updates helper lambdas and setters/getters to EveSpaceObjectChild. |
| trinity/Eve/EveEffectRoot2.h | Migrates effect-children owner API and storage types to EveSpaceObjectChild. |
| trinity/Eve/EveEffectRoot2.cpp | Maintains owner pointers for effect children on init/list events/destruction. |
| trinity/Controllers/Actions/Tr2ActionPlayCurveSet.cpp | Drops now-unused legacy child include. |
| trinity/Controllers/Actions/Tr2ActionChildEffect.h | Migrates stored child pointer type to EveSpaceObjectChildPtr. |
| trinity/Controllers/Actions/Tr2ActionChildEffect.cpp | Loads child effects as EveSpaceObjectChild. |
| trinity/CMakeLists.txt | Adds new base-class source files to the build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use `UnregisterChild` helper method Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
|
Ikreb1
requested changes
Jul 20, 2026
| MAP_METHOD_AND_WRAP( "StartControllers", StartControllers, "Start all controllers" ) | ||
| MAP_METHOD_AND_WRAP( "GetMethodVariableName", GetMethodVariableName, "Get the name of the seed variable being utilized" ) | ||
| MAP_METHOD_AND_WRAP( "SetProceduralContainerVariable", SetProceduralContainerVariable, "Set variable for all applicable ProceduralContainers\n:param name: variable name\n:param value: new variable value\n" ) | ||
| MAP_PROPERTY_READONLY( "partTag", GetPartTag, "Part tag for multi-part space objects" ) |
Author
There was a problem hiding this comment.
The reason is because of the "name" attribute. If we use EXPOSURE_CHAINTO the order of attributes would change: the "name" attribute, which is normally the first one, would move to the bottom. It is not a problem for the Graphite editor because it already pulls the "name" attribute to the top, but it will create noise in .red files as it maintains the attribute order, so loading and saving a .red file would introduce a modification. I want to avoid it.
|
|
||
| MAP_ATTRIBUTE( "cellScreenSize", m_cellScreenSize, "Target size of a single cell in a grid on the screen (in pixels)", Be::READWRITE | Be::PERSIST ) | ||
| MAP_ATTRIBUTE( "currentLod", m_currentIB, "Current cloud LOD", Be::READ ) | ||
| MAP_PROPERTY_READONLY( "partTag", GetPartTag, "Part tag for multi-part space objects" ) |
Member
There was a problem hiding this comment.
Do all child.* classes really need to expose the partTag? wont this incentivise people to misuse this parameter as a second name override? maybe that is fine and good but I suspect we will get alot of confusing code in python
Author
There was a problem hiding this comment.
It is a read-only property that would be assigned by the modular object builder, so it would be fairly hard to abuse.
| child->SetOwner( owner ); | ||
| } | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
should these functions be the default implementation in the base class or are there too many things that would do nothing and should those overwrite the function with empty instead maybe?
Author
There was a problem hiding this comment.
The default implementation EveSpaceObjectChild::SetOwner just changes its own "owner" pointer. But any classes that maintain their children would need to override it to propagate changes.
The alternatives are:
- Have children list in
EveSpaceObjectChild: essentially any child object may have children. On the bright side, all child management can be done in a single place. But it would come at a cost of extra object size (and most of our child classes are leaves with no children) - Have something like
EveSpaceObjectChild::EnumerateChildrenvirtual method that child classes that manage children would override to provide their children. Then theEveSpaceObjectChildwould be able to manage all of these pointers for its children as well.
| * @param query Type of bounding sphere query. | ||
| * @return True if the bounding sphere is valid, false otherwise. | ||
| */ | ||
| virtual bool GetBoundingSphere( Vector4& sphere, BoundingSphereQuery query ) const; |
Member
There was a problem hiding this comment.
Why does this one not default to EVE_BOUNDS_NORMAL like the original behaviour for some of the classes?
Author
There was a problem hiding this comment.
Default arguments and virtual functions don't mix well. At the very least, we get a clang-tidy warning about it. For a lengthy explanation, see https://www.sandordargo.com/blog/2021/01/27/virtual-functions-with-default-arguments for example
| m_name = BlueSharedString( name ); | ||
| } | ||
|
|
||
| void EveSpaceObjectChild::UpdateVisibility( const EveUpdateContext&, const Matrix&, Tr2Lod ) |
Member
There was a problem hiding this comment.
wouldnt it be better if these empty functions were declared inline in the header to simplify reading in the future of what has and hasn't been implemented with any base class behaviour?
Author
There was a problem hiding this comment.
Not according to our coding standards
| { | ||
| child->SetParent( this ); | ||
| child->SetOwner( m_owner ); | ||
| if( m_partTag != EveSpaceObjectChild::NO_PART_TAG ) |
Member
There was a problem hiding this comment.
m_partTag != EveSpaceObjectChild::NO_PART_TAG && child.m_partTag != EveSpaceObjectChild::NO_PART_TAG
or are we expecting children to inherit the part tag of the parent by default?
Author
There was a problem hiding this comment.
The part tag, if it's set, should be inherited. The only reason to not propagate NO_PART_TAG is performance: if the object was built/loaded by anything other than the modular object builder, we shouldn't pay the price of managing these tags.
It may break if we start moving children in the hierarchy, but we shouldn't be doing that.
…hatever was there before (#30)
The new base class has support "owner", "parent" and "part tag" shared attributes.
Use `UnregisterChild` helper method Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…rbonengine/trinity into space-object-child-base-class
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
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.
The base class is long overdue to manage the common state between all child object classes.
The new base class supports "owner", "parent," and "part tag" shared attributes.
The owner is the space object owner. It can be queried from C++ or Python at any time. This will simplify picking code in the client.
The parent is the parent space object child of the given child. Similarly, it can be queried from C++ and Python. This one is useful for the editor to speed up the search of the transform parent of an object.
The part tag is currently unused; it is going to be needed for the modular space objects to identify which logical part of the object the child belongs to.
Special care must be taken to maintain owner and parent pointers in child objects, to make sure they are always up to date (internally, these are stored as raw pointers because we can use strong pointers as they would create circular references, and we should avoid weak pointers because of performance issues with BlueWeakRef). Any child object that owns other children needs to maintain the owner/parent state for its children.