-
Notifications
You must be signed in to change notification settings - Fork 4
Material property management #24
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
base: main
Are you sure you want to change the base?
Conversation
...to a "push" architecture
presumably, multiple properties will be set
@gotmachine: I've been working on this for a while. Do you think it better belongs here, or in KSPCF? It does touch/fix stock issues, but I feel like the approach is too invasive/not enough of a 1:1 replacement for KSPCF. |
Well TBH I think Shabby as a whole could be part of KSPCF. Both mods have broadly similar goals and are touching similar stuff in the same way. As for this specific PR, you're likely better informed than me to make that call. I'm not opposed to radical modifications to the stock implementations, as long as stock end user facing stuff is untouched and that the modding ecosystem fallout is assessed and either inexistant or manageable from our side. |
Yeah that's fair re. Shabby/KSPCF overlapping. In this particular instance my main hesitation is that any mod which directly writes to Mods applying their own MPBs to the part hierarchy will also break, but I am not as concerned about this, because it's rather difficult to do that correctly and this is intended to address that. Technicolor will be the first external consumer of this API. This seems to be a bit too much breakage for my liking, but if you think differently we can move it. But in either case, I would appreciate a code review from you if you have time, particularly around Harmony patching and garbage management. |
Shabby's scope has certainly expanded, and some of the newer stuff does seem like a better fit in KSPCF. Can those new parts be cleanly removed from the rest of Shabby (Shader.Find replacement) ? But ultimately inertia will probably win out here and we continue down the current path; that's fine too. |
Yes, |
Note: the material duplication patch should be moved to KSPCF before merging. |
A complete overhaul of how dynamic shader properties are handled for parts. Rewrites how vanilla uses (read: attempts to use, largely incorrectly)
MaterialPropertyBlock
s, and also provides an API for other mods to apply their own MPB-based changed without overwriting the vanilla features.In brief, vanilla uses a single part-level MPB to implement the following:
Part
;MaterialColorUpdater
(which, astoundingly, accidentally instantiates all of a part's materials due to callingRenderer.material
instead ofsharedMaterial
);HighlightingSystem
, independent of the part's mesh);ModuleColorChanger
s. How this works given that the MPB is never reset is unclear to me.This PR Harmony-patches all of the above usages to become consumers of the new API.
Custom properties are specified using instances of the
Props
class, which offers an MPB-like API. In addition, there is aPriority
field, which determines precedence when multipleProps
set the same property on the same renderer.There is a singleton class
MaterialPropertyManager.Instance
, which can be used toRegister
aProps
class to be applied to a particular renderer. This only needs to be done once per renderer, and a single instance ofProps
can be registered for any number of renderers. Any subsequent changes made to thatProps
instance will be transparently applied to its registered renderers, inLateUpdate
. Reusing a singleProps
for multiple renderers is encouraged, since all renderers sharing the same set ofProps
will share the same underlying MPB.Multiple
Props
registered against the same renderer will be combined, with conflicts resolved by order of highestPriority
. Properties set by vanilla code, as listed above, have priorityint.MinValue + 1
, except for those set by MCC, which have priority0
.