Kylemc1413 / SongCore

A plugin for handling custom song additions in Beat Saber.
MIT License
87 stars 48 forks source link

Add missing alpha value in color #131

Closed KivalEvan closed 6 months ago

KivalEvan commented 7 months ago

somehow this got slipped past for long while, figured why my light was oddly dim ingame vs editor

shouldnt be an issue for MapColor a value seeing it has already been handled elsewhere

Meivyn commented 6 months ago

Would you mind telling me what this solves (preferably with an image)? The default alpha is 1, so it shouldn't be dimmed. I'm also not sure to know how alpha affects the map colors (didn't play with that myself).

KivalEvan commented 6 months ago

since a was introduced for custom color (which official color scheme supports), the value from both of the custom color and color scheme did not get used; unlike vanilla game behaviour which does read and applied as intended

Meivyn commented 6 months ago

Official color schemes technically support alpha value just because they use the Unity Color struct. The UI to set color schemes doesn't.

image

You can see how broken it looks when setting the alpha value close to zero, it doesn't have any effect on notes, though it messes up very badly with trails and the glow in general. That value seems to only work properly on the environment colors.

Because of this, I'd rather add an overload and use that on the environment colors instead. Or add an optional parameter with a default value of 1. I feel like it might be more annoying than anything and might confuse people, they would end up in help channels asking why their bloom is gone...

KivalEvan commented 6 months ago

color scheme alpha alteration is also used for user color override (which certain ppl use to make wall appear differently), idk about notes if anyone really tried but personally would be interesting to see creative use of it

it's just uncommon use case which can be very handy when needed, would agree that it should only be applied to environment color if the other thing like note and obstacle may deem unintended behaviour for reasons, ultimately it's up to mapper to decide this

Pink seems to have done work to ensure that alpha do get default value of 1, and through testing, i don't see it being a problem with the way it currently is

Meivyn commented 6 months ago

Guess I'll see how it goes. I hope mappers won't cause a headache to support.