mapeditor / rs-tiled

Reads files from the Tiled editor into Rust
https://crates.io/crates/tiled
MIT License
268 stars 103 forks source link

`ObjectLayerData::colour` shouldn't be an Option #175

Closed aleokdev closed 2 years ago

aleokdev commented 2 years ago

From https://doc.mapeditor.org/en/stable/reference/tmx-map-format/#objectgroup: The member should default to #a0a0a4. Apart from this, it should be renamed to color for consistency and there should be a property to access it from ObjectLayer.

bjorn commented 2 years ago

Ah, I don't think we need the #a0a0a4 as actual default value. This is a bit of an error in the documentation, because actually the default is an invalid color, which will get rendered as Qt::gray by Tiled:

QColor MapObject::effectiveColor() const
{
    const QString effectiveType = this->effectiveType();

    // See if this object type has a color associated with it
    for (const ObjectType &type : Object::objectTypes()) {
        if (type.name.compare(effectiveType, Qt::CaseInsensitive) == 0)
            return type.color;
    }

    // If not, get color from object group
    if (mObjectGroup && mObjectGroup->color().isValid())
        return mObjectGroup->color();

    // Fallback color
    return Qt::gray;
}

So I think using Option makes sense here.