Conversation
Hoikas
left a comment
Hoikas
left a comment
There was a problem hiding this comment.
Another review will be incoming when I have a chance to look at the UI in Blender.
korman/exporter/mesh.py
Outdated
|
|
||
| # Attach a FogEnvironment if it's non-default | ||
| if mods.fogenv.fog_env and mods.fogenv.fog_env != bpy.context.scene.world: | ||
| geospan.fogEnvironment = self._exporter().mgr.find_create_key(plFogEnvironment, bl=bo, name=mods.fogenv.fog_env.name) |
There was a problem hiding this comment.
self._mgr is a shortcut for this
korman/exporter/mesh.py
Outdated
| geospan.props |= plGeometrySpan.kPropRunTimeLight | ||
|
|
||
| # Attach a FogEnvironment if it's non-default | ||
| if mods.fogenv.fog_env and mods.fogenv.fog_env != bpy.context.scene.world: |
There was a problem hiding this comment.
Considering all the access of mods.fogenv, it would be nice to cache that value for the (small) performance benefit and less verbosity.
| "exp2": plFogEnvironment.kExp2Fog | ||
| } | ||
|
|
||
| class PlasmaFogEnvMod(PlasmaModifierProperties): |
|
|
||
| def export(self, exporter, bo, so): | ||
| if self.fog_env is None or self.fog_env == bpy.context.scene.world: | ||
| return # Don't generate a FogEnv for the default Age FNI fog |
There was a problem hiding this comment.
Comments should appear on their own lines
| fe = exporter.mgr.find_create_object(plFogEnvironment, bl=bo, name=self.fog_env.name) | ||
| env = self.fog_env.plasma_fni | ||
|
|
||
| fe.type = fog_types[env.fog_method] |
There was a problem hiding this comment.
The preferred method for doing this is to have blender store the attribute name of the constant and use getattr to apply it.
There was a problem hiding this comment.
I don't think I can do that in this case because the plasma_fni enum values don't match the names of the constants, but are already in use in existing data :(
korman/ui/modifiers/render.py
Outdated
| # Other settings | ||
| layout.prop(modifier, "replace_normal") | ||
|
|
||
|
|
There was a problem hiding this comment.
Methods should only have one line of whitespace separating them (see PEP8)
korman/ui/modifiers/render.py
Outdated
|
|
||
| col = split.column() | ||
| row = col.row() | ||
| row.active = fni.fog_method == "linear" |
There was a problem hiding this comment.
Korman prefers to hide unused properties.
There was a problem hiding this comment.
Hiding the start field caused some annoying visual jumping of the end/density fields when toggling between fog types. I tried to work around it by adding a blank space if we didn't need the start field, but apparently Blender ignores that :\
korman/ui/modifiers/render.py
Outdated
| layout.prop(modifier, "replace_normal") | ||
|
|
||
|
|
||
| def fogenv(modifier, layout, context): |
| bl_description = "Adjust per-object fog settings" | ||
| bl_icon = "MAT_SPHERE_SKY" | ||
|
|
||
| fog_env = PointerProperty(name="Environment", |
There was a problem hiding this comment.
This seems like a misnomer when you are referencing a world. It's also confusing when accessing bo.plasma_modifiers.fogenv.fog_env because of the redundant naming and inconsistent spelling.
|
Updated. Only thing I didn't address was the lookup table for the attribute names, because I think we need to keep consistency with the existing |
|
I've been sitting on this for awhile because I'm not sure how much I like creating a new ID block in Blender to store fog environments. I'm thinking it might be better to refactor the way fogs are done from the ground up. Namely, instead of having the fog properties stored directly on the world object, we should have a collection of fog properties on the world. I would argue the default fog environment should work like the Default page does, we default to either no fog or D'ni fog but allow the author to override this by creating a nameless or "Default" fog. |
2 participants
Adds a new Fog Environment modifier to allow per-object fog settings. Multiple objects can reference the same FogEnvironment.
This is implemented by creating a new Blender World for each fog environment and reusing the existing
plasma_fniproperties. This works without changing the default World associated with the Scene, so the global fog settings remain as they were.On the subject of global fog settings, if you try to add the global World as a fog environment to an object, it is silently ignored to avoid polluting the PRP files with unnecessary keys. Hypothetically there's a potential optimization where we could avoid adding the fog environment ref if the material is flagged as NoFog/ReallyNoFog.
Worth mentioning that PRP-based FogEnvironments are static and cannot be animated or modified by Python.