-
Notifications
You must be signed in to change notification settings - Fork 280
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
LOD Generator Helper Presets #63
base: master
Are you sure you want to change the base?
Conversation
LODGeneratorHelper has two new properties, A LODGeneratorPreset obj ref and a "customization" toggle. If customization is enabled (default), the helper drives all properties itself. If customization is disabled, properties are driven by a specified preset. If no preset is specified, defaults matching previous behavior are used.
When a LOD Generator Helper has customization disabled, Properties driven by the specified preset are visible but greyed out.
Code between calls EditorGUI.BeginDisabledGroup and EditorGUI.EndDisabledGroup scoped for clarity.
Properties not driven by a preset are listed first. This way, there's only a single block of greyed-out properties when customization is disabled.
A bit of background: an artist on my team wanted the ability to specify generic LOD settings for things like "Large Prop", "Small Prop", "Player Character", etc. We both got tired of copying helper components around, so I figured adding referencable presets would be useful. |
Thanks for the PR! |
I like the idea behind this, but I'm not sure that this is the behavior that I'd expect and want myself. I think that it would be better if instead of just copying over the settings from the preset asset, the settings from the preset asset are used directly instead. This way you can update the preset, and those settings are used anywhere it's referenced automatically. It could still be done so that you can load preset settings and then customize it for each instance, but this would mean that the preset link would be broken. But that wouldn't be different from the behavior of this PR right now. I think that'd be acceptable and expected. Then you wouldn't need to have a checkbox to customize settings. You simply use a preset, or not (which means customize on instance level). The downside is that the automatic collection of renderers is required then, unless it's possible to specify those in addition to using the presets for everything else per LOD level 🤔 What do you think? |
This would definitely be a better behavior! Customize checkbox or not, it should probably work this way and I can update the PR so That said: regardless of whether LOD generation pulls from a preset's properties or properties on the helper, LODs won't be regenerated on a property change (currently you need to open the LOD'd prefab, destroy existing LODs from the helper, and regenerate them). My expectation is for a prefab's LODs to be automatically regenerated when its helper's settings or ref'd preset change; my plan was to tackle this in a subsequent PR.
So if I'm understanding this right, the idea is that a There's some advantage to retaining a reference to the preset you used as a "base" before customization, in case you want to revert your customizations but don't remember which preset you used. Though in practice, you'll probably be choosing from a small number of presets and could guess based on the asset, so it might not be as useful as I'm assuming 😄 Another advantage to retaining the preset ref on a customized asset is for validation. On my current project, we plan to validate that customized settings aren't "too" customized from a known preset. When a given preset is used as a base, the same # of levels must be used but a few settings (eg. quality, transition height, and shadow casting) can be tweaked within some range. The thought being, if you want to deviate far from an existing preset then you should probably create a new preset and use that instead. This is obviously a validation requirement specific to my project (and so not a part of this PR), but it's not possible unless a ref to the preset assigned before customization is retained.
Ultimately, I think the value of explicitly toggling customization is in clarifying the user's intent. My model for this is in how Unreal handles property overrides on material instances; you have to explicitly enable customization of a given property, so when a parent material's properties are changed the instance receives un-overridden changes while retaining local overrides. Notably, you can enable customization of a property without changing its value! Toggling customization without overriding values acts to say "I like the current values of my base, but if my base changes I want to keep using the old values", which is potentially useful.
This was also part of the reason I opted to copy settings over when setting the preset, yeah. The Renderer list could be pulled out of I think your intuition is correct that for uncustomized helpers, the ref'd preset's settings should be used directly. I do feel that holding on to the preset ref after customization is useful for validation and reversion, and explicitly toggling customization clarifies the user's intent. |
Thinking about this again, I don't think it's necessary anymore. Because you only need the settings when you generate the LODs. So maybe it's just enough to copy it over again when generating the LODs instead of just during OnValidate like now.
Right. This would be good. But definitely a bonus. I think it could probably be a button on the preset asset since it would be a more heavy operation potentially. Like "Apply changes to all prefabs" or something. But as you said before, it'd be good not to introduce any additional dependencies just for that.
Yeah, you're probably right. Keep the toggle with a base reference.
Yeah, skip that. I was only thinking about possibilities, but you're right that it complicates things. |
Related issues
No related issues AFAIK.
Describe the changes
This PR introduces the ability to define sharable/reusable LOD generation presets. A new ScriptableObject type,
LODGeneratorPreset
defines most LOD generation settings.LODGeneratorHelper
s can reference a preset, which will override the helper's settings. Additionally, you can enable customization on an asset-by-asset basis if you need to tweak any settings beyond what a given preset provides.Some properties (Auto-collect renderers, override save path, and specified renderers) remain helper-driven rather than preset-driven.
To ensure backwards-compatibility,
LODGeneratorHelper
s default to customization enabled, no preset specified. Disabling customization will "snap" that helper's settings to the specified preset. If no preset is specified, the plugin's traditional default LOD settings will be used.How to test
Additionally, LOD Generator Helpers created before presets were introduced should function identically as before (previously set settings should not have changed, LODs should generate the same).
Screenshots
Screenshots and video!
The preset scriptable object:
New settings on LOD Generator Helper
Preset selection (note that preset-driven properties are greyed out, and the component's properties have been updated to match the preset)
Video showing flow:
https://user-images.githubusercontent.com/103901155/180583699-5708715f-bed8-443f-860c-b92cb26c8f1a.mp4
Known issues
If the properties of a
LODGeneratorPreset
are updated, objects referencing that preset are not automatically updated (nor are their LODs regenerated). This would require searching the asset database for objects referencing a changed preset. Unity's search extensions package (https://github.com/Unity-Technologies/com.unity.search.extensions) makes this straightforward, but I didn't want to introduce the dependency. In the meantime, manually enabling & disabling customization on anyLODGeneratorHelper
s ref'ing a changed preset will update that helper's properties.Further details
🤷♂️