renderOrder to RenderOrderComponent objectLayers to ObjectLayerComponent #9472
Conversation
|
Thank you for your pull request and welcome to the Ethereal Engine developer community! We require contributors to sign our Copyright Assignment Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the agreement at https://forms.gle/15ENsSAJGKf2ozvB7 The agreement has not been signed by users: @MichaelEstes. After signing the agreement, you can ask me to recheck this PR by posting |
|
@cla-bot check |
|
Checking that all contributors to this PR are verified. |
| for (const layer of component.objectLayers.value) { | ||
| object.layers.enable(layer) | ||
| } | ||
| } |
There was a problem hiding this comment.
In the prior objectLayer implementation we were running a function updateWorldObjectLayers before when seting Object layer. I'm not familiar with the purpose myself but I see it missing from this implementation. There was likely a reason why it's there @HexaField
There was a problem hiding this comment.
The purpose of that function was to update lists on Engine.instance.objectLayerList. My preference is that we deprecate all that, and introduce a tag for each layer. In other words:
const ObjectLayerComponents = [defineComponent({name: 'ObjectLayer0'}), ..., defineComponent({name: 'ObjectLayer31'})]
export const ObjectLayerComponent = defineComponent({
...,
setObjectLayers: (entity:Entity, ...layers: number[) => {
layers.map((i) => setComponent(entity, ObjectLayerComponents[i]);)
},
enableObjectLayer: (entity:Entity, layer: number, enable: boolean) {
layers.map((i) => setComponent(entity, ObjectLayerComponents[i]);)
}
})
|
|
||
| onInit(entity) { | ||
| return { | ||
| renderOrder: 0 |
There was a problem hiding this comment.
If you wanted you could probably just set the return as the number value and instead of passing {renderOrder: 1} in set component you can just pass 1
export const ObjectLayerComponent = defineComponent({
...,
entitiesBylayer: Record<layer: number, Entity[]>
}) |
Attaching data to components like this is not ideal; this does not get cleaned up automatically between tests. If we want to store data outside of components, we should use |
| name: 'ObjectLayerComponent', | ||
| schema: { mask: Types.i32 }, | ||
|
|
||
| onSet(entity, component) { |
There was a problem hiding this comment.
let's add the mask here as the input args, and add a toJSON() that returns the mask, so that this component is serialized properly.
| }) | ||
| }) | ||
|
|
||
| export const ObjectLayerComponent = defineComponent({ |
There was a problem hiding this comment.
Maybe rename this to ObjectLayerMaskComponent for clarity, and to distinguish from the individual layer components?
| return 0 | ||
| }, | ||
|
|
||
| onSet(entity, component, renderOrder) { |
There was a problem hiding this comment.
This needs a similar refactor as the object layers, w/ the renderOrder property relying on this data as the source of truth
| import { GroupComponent } from './GroupComponent' | ||
|
|
||
| export const RenderOrderComponent = defineComponent({ | ||
| name: 'RenderOrderComponent', |
There was a problem hiding this comment.
let's add a bitecs schema for this as well.
…ent (#9472) * Start of RenderOrderComponent * Change RenderOrderComponent to set render order on objects in group * Change file locations * Start of ObjectLayerComponent and tests * Update setObjectLayers to LayerObjectComponent * Update renderOrder to RenderOrderComponent * add comments for each layer * RenderOrderComponent changes, ObjectLayerComponent WIP * Save for later * Update ObjectRenderComponent, revert back to setObjectLayers, added Layer tests * Add setter for Layer, change uint to int (Three expects -1 for enable all), add tests * Change render order to use object property * ObjectLayerMaskComponent toJson, rename, mask as set param * Update toJson * Change RenderOrder from class to function * Cleanup * Fix tests and remove Engine.instance.objectLayerList * Update RenderOrderComponent.test.tsx --------- Co-authored-by: HexaField <joshfield999@gmail.com> Co-authored-by: Gheric Speiginer <gheric.speiginer@gmail.com>
Summary
Replaces setting renderOrder and objectLayers with RenderOrderComponent and ObjectLayerComponent
removes
deprecates
References
closes #9327, #9328
QA Steps