Conversation
Adds a Color property on MapObject that takes priority over the object layer's color but is overridden by a Class color: Class > Object Color > Layer Color > gray default.
places that manually copy MapObject fields, causing it to silently reset to unset whenever an object was cloned, saved as a template, or synced with its template on placement."
There was a problem hiding this comment.
Thanks for this contribution! Some things need to be done before this can be merged:
- Lua plugin needs to export object color
- Scripting API needs to be extended (
EditableMapObject) docs/map.xsdcould be updated (maybe alsodocs/map.dtdbut it's very stale anyway)
Documentation needed:
docs/reference/tmx-map-format.rst: addcolorto the<object>attribute listdocs/reference/tmx-changelog.rst: changelog entry for the new attributedocs/reference/json-map-format.rstaddcolorto the object field table and its changelog- Update
NEWS.md(e.g. "Added support for overriding the color of individual map objects (by BlueAlter, #4610)")
In general I do wonder if this feature is really meaningful. There's also still a request open to add a per-object tint color, which I think would be more generally useful. What made you need this per-object color specifically?
| if (auto classType = Object::propertyTypes().findClassFor(effectiveClassName(), *this)) { | ||
| colors.main = classType->color; | ||
| drawFill = classType->drawFill; | ||
| } else if (mColor.isValid()) { |
There was a problem hiding this comment.
Is it on purpose that the per-object color does not override a class-based color? It seems counter intuitive. I think per-object color should override both per-layer color and per-class color.
| TemplateProperty = 1 << 13, | ||
| CustomProperties = 1 << 14, | ||
| ColorProperty = 1 << 15, | ||
| AllProperties = 0xFF |
There was a problem hiding this comment.
Whoops, this AllProperties does not include ColorProperty but actually also misses everything starting from SizeProperty (existing issue). Probably needs fixing.
| AllProperties = 0xFF | |
| AllProperties = 0xFFFF |
| } | ||
|
|
||
| const QString colorString = atts.value(QLatin1String("color")).toString(); | ||
| if (!colorString.isEmpty()) { |
There was a problem hiding this comment.
Let's check QColor::isValid instead of QString::isEmpty so we don't treat invalid color strings as an overridden color (also in varianttomapconverter.cpp).
| if (shouldWrite(!mapObject.isVisible(), isTemplateInstance, mapObject.propertyChanged(MapObject::VisibleProperty))) | ||
| w.writeAttribute(QStringLiteral("visible"), QLatin1String(mapObject.isVisible() ? "1" : "0")); | ||
|
|
||
| if (shouldWrite(mapObject.color().isValid(), isTemplateInstance, mapObject.propertyChanged(MapObject::ColorProperty))) |
There was a problem hiding this comment.
We should avoid writing invalid colors, which would get saved as black:
| if (shouldWrite(mapObject.color().isValid(), isTemplateInstance, mapObject.propertyChanged(MapObject::ColorProperty))) | |
| if (mapObject.color().isValid() && shouldWrite(true, isTemplateInstance, mapObject.propertyChanged(MapObject::ColorProperty))) |
You've already done this for the JSON format.
This branch has not been deployed
2 participants
Adds a Color property on MapObject that takes priority over the object layer's color but is overridden by a Class color: Class > Object Color > Layer Color > gray default.