llealloo / audiolink

Audio reactive prefabs for VRChat
Other
354 stars 40 forks source link

ThemeColorEnable -> ThemeColorMode #172

Closed DomNomNom closed 2 years ago

DomNomNom commented 2 years ago

I found the existing checkbox to be misleading as it turning it off still made theme colors show up but they are based off the CC colors. This also allows more modes to be added in the future. One I'd like to add is to have the colors based off a video screen.

Before: before

After: image

pema99 commented 2 years ago

That is less confusing, indeed.

I'm not a fan of including that extra StringInListDrawer script unless it's really necessary. Can you not just use an enum make the dropdown?

Also, I would probably change the names to "ColorChord colors" and "Custom" if it were up to me, but that's a bit of a nitpick. People may not know the CC abbreviation.

pema99 commented 2 years ago

This also allows more modes to be added in the future.

Cool :D

One I'd like to add is to have the colors based off a video screen.

IMO bordering on out of scope for AudioLink, though the same can be said for other features that have gone in in the past. Wonder how you would do that in a performant manner. Also, if you do want to add something like that, it definitely needs to work outside of VRChat too (probably just by compiling it out with some guards when not compiling for VRC).

DomNomNom commented 2 years ago

I'm not a fan of including that extra StringInListDrawer script unless it's really necessary. Can you not just use an enum make the dropdown?

I agree an enum would be better. But it doesn't seem to be possible right now as I'm getting this error:

[UdonSharp] Assets/AudioLink/Scripts/AudioLink.cs(74,8): System.NotSupportedException: UdonSharp does not yet support user defined enums

"ColorChord colors" and "Custom"

I also like those names better.

IMO bordering on out of scope for AudioLink

Given that theme colors are in, I think this is kind of mechanism is useful to have a centralized way of getting the theme colors. Having this logic be centralized would avoid having this kind of code be run multiple times elsewhere.

Wonder how you would do that in a performant manner

This is not set in stone but I'm thinking of trying it this way:

This might be possible purely in shaderland, spreading sorting over multiple frames. (sorting doesn't have to be perfect) Might imply that there's a few frame delay but I think it would work well enough.

pema99 commented 2 years ago

Oh, I guess enum's are only a U# 1.0 thing. Annoying. Does the script at least use a compatible license?

pema99 commented 2 years ago

I'll verify these changes locally when I next have time.

DomNomNom commented 2 years ago

Does the script at least use a compatible license?

Yes, the author says it's MIT licensed https://gist.github.com/ProGM/9cb9ae1f7c8c2a4bd3873e4df14a6687?permalink_comment_id=3863200#gistcomment-3863200

DomNomNom commented 2 years ago

I added the "custom" to the input theme colors to distinguish them from the output theme colors which depend on the theme color mode.

I updated the description with an updated screenshot. I think I've hit some kind of github caching bug where unless I view it in a new private browser window I still see the old image for the "after".

pema99 commented 2 years ago

Seems to be working great. Thanks