-
Notifications
You must be signed in to change notification settings - Fork 92
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
Implement Terrain:GetMaterialColor and Terrain:SetMaterialColor #93
Implement Terrain:GetMaterialColor and Terrain:SetMaterialColor #93
Conversation
From<Color3> for DomCoolor3uint8 was not correct here, and these conversions are already implemented by rbx-dom anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple nitpicks with error messages / naming, other than that this looks great and is also really consistent with the rest of the codebase. Good catch with the color conversions too 👍
Co-authored-by: Filip Tibell <[email protected]>
Co-authored-by: Filip Tibell <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Thanks! |
Closes #90 by implementing the
Terrain:GetMaterialColor
andTerrain:SetMaterialColor
methods.I decided not to support passing strings to the material argument. This would require a bit more rigamarole to convert the string to an enum, and iirc Roblox recommends using enums over strings anyway. I'm open to adding it if complete parity with Roblox's implementation is important
I also decided to switch Lune's
Color3
<->Color3uint8
conversions so they just call rbx-dom's implementations, because Lune'sFrom<Color3> for DomColor3uint8
was not quite correct, and they might as well use what's already there