-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Where to keep default values #307
Comments
Philosophically, it sounds more like something a system (render) should worry about. |
Practically, not having a fully formed component before the first system pass is an issue. |
As mentioned in #309 if we remove
how does one discovers a) docs? system api comments? |
Maybe the problem is not the component constructors but the name spacing of them. Maybe needing a reference to Standalone systems work well (if you know which ones needs ctx reference) module.exports = (node, graph) => {
const { systems } = require("pex-renderer");
const triggerIn = node.triggerIn("in");
const triggerOut = node.triggerOut("out");
const renderSystem = systems.render({ ctx: graph.ctx });
triggerIn.onTrigger = (props) => {
renderSystem.update(props.entities, props.deltaTime);
node.comment = props.entities.length;
triggerOut.trigger(props);
};
}; Maybe the same could work with components import { components as cmp, entity as createEntity } from "pex-renderer";
const entity = createEntity({ //assigns unique id based on built-in entity counter
//assigns identity modelMatrix and empty worldBounds
transform: cmp.transform({ position: [0, 0, 1] }),
//assigns default roughness, metallic and sets all textures to null
material: cmp.material({ baseColor: [1, 1, 1, 1] })
}) It gets a bit more problematic with components that need import { components as cmd, entity as createEntity } from "pex-renderer";
const entity = createEntity({
//assignes reflectionOctMapAtlas to null
reflectionProbe: cmp.reflectionProbe()
}) So we know it's Why still pushing for constructors / factory functions. With e.g. |
There are few variable here and there that if not set might create unexpected or black renders:
default
roughness: 1
andmetallic: 1
are necessary evil in case you provide metallic / roughness texture (ideally they would be roughness=1 and metallic=1emissiveIntensity
has to be set (default to at least 1)clearCoatRoughness
should be at least 0.04In the current 2.0 (pre-ECS) implementation those values are scattered across material and renderer with various
value || default
checks. That becomes a problem even more with ECS where material could be empty object. I wonder what could be unified approach to handling those.render-system
'sentity._material
or maybe even someentity._material.uniformCache
pex-shaders
and ifdef everything if not explicitly set in the entity.material e.g.clearCoatRoughness
would requireUSE_CLEAR_COAT_ROUGHNESS
oremissiveIntensity
would needUSE_EMISSIVE_INTENSITY
instead of defaulting to forgottenuEmissiveIntensity=0
.The text was updated successfully, but these errors were encountered: