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

[Draft] Added missing EditableWangSet::SetColorImageTile and EditableWangSet::SetColorProbability #4070

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Racines
Copy link

@Racines Racines commented Oct 5, 2024

It's seems there is no way in the current api to edit wangcolor tile preview and probability: https://discord.com/channels/524610627545595904/524610627545595906/1292100220569653248

I added them in this PR but this has not been tested.
However, after a discussion with eishiya on discord, a better way to expose these features is described here: #3900

@eishiya
Copy link
Contributor

eishiya commented Oct 5, 2024

(Repeating what I wrote on Discord, but hopefully more coherent :)

I think it would be best to have an EditableWangColor that has the standard TiledObject methods for setting custom properties, and has color (the Terrain color), name, imageTile, probability, wangSet, and id (its index within its WangSet). This would make interacting with WangColor more like interacting with Tiles in a Tileset, or Layers in a TileMap.

WangSet's setColorName() and colorName() would be deprecated, and instead WangSet should get methods similar to those of Tileset to fetch WangColors, e.g. colors could return WangColor[], and color(id) could return the WangColor with that id, and editing the name and other properties would be handled via the WangColor, not the WangSet.

This PR, though useful (assuming it works xP), continues the current pattern of dealing with individual Wang colours via their WangSet. While that works and it's hard to argue with adding missing functionality, it's a different pattern from how similar entities work elsewhere in the API, and implementing all the methods for handling custom properties using this same approach would be a mess (setColorColorProperty(id, color) anyone?).

Wang colours have way too many properties useful to scripting to be treated as just a number within WangSet, and I think the API should reflect that. And the sooner these corrections are made, the fewer scripts will need adjustments, and the less functionality will need to be rewritten by whoever tackles this structural change.

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.

2 participants