Skip to content
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

Merged

Conversation

kennethloeffler
Copy link
Contributor

Closes #90 by implementing the Terrain:GetMaterialColor and Terrain: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's From<Color3> for DomColor3uint8 was not quite correct, and they might as well use what's already there

Copy link
Collaborator

@filiptibell filiptibell left a 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 👍

src/roblox/instance/terrain.rs Show resolved Hide resolved
src/roblox/instance/terrain.rs Outdated Show resolved Hide resolved
src/roblox/instance/terrain.rs Outdated Show resolved Hide resolved
src/roblox/datatypes/types/enum_item.rs Outdated Show resolved Hide resolved
src/roblox/datatypes/types/color3.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@filiptibell filiptibell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@filiptibell filiptibell merged commit bcfc7d2 into lune-org:main Aug 24, 2023
@kennethloeffler kennethloeffler deleted the terrain-get-set-material-colors branch August 24, 2023 23:39
@hoontee
Copy link

hoontee commented Aug 24, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support modifying Terrain MaterialColors / implement SetMaterialColor and GetMaterialColor
3 participants