tiled icon indicating copy to clipboard operation
tiled copied to clipboard

Added "Tileset > Edit Tileset Image Parameters..." action

Open bjorn opened this issue 1 year ago • 3 comments
trafficstars

This makes the functionality easier to discover. Previously editing the tileset image parameters was only possible by clicking the value of the Image property in the Properties view, and then the appearing "Edit" button.

bjorn avatar Aug 15 '24 11:08 bjorn

@CodiumAI-Agent /review

(just curious what feedback it might have)

bjorn avatar Aug 15 '24 11:08 bjorn

PR Reviewer Guide 🔍

(Review updated until commit https://github.com/mapeditor/tiled/commit/c8a7cdaaa969e62e7b3c514209201a895a9c980e)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Update Consistency

The new action for editing tileset parameters is integrated using updateActions instead of the former updateAddRemoveActions. Please verify that all UI state changes (such as enabling/disabling actions) are consistently applied in all contexts where the state update is needed.

    updateActions();
}
Default Initialization

The TilesetParameters struct now includes a default constructor and default initializers for tileSpacing and margin. Confirm that these default values are appropriate across all usages and do not unintentionally affect behavior when comparing parameter changes.

    TilesetParameters() = default;

    explicit TilesetParameters(const Tileset &tileset);

    bool operator != (const TilesetParameters &other) const;

    QUrl imageSource;
    QColor transparentColor;
    QSize tileSize;
    int tileSpacing = 0;
    int margin = 0;
};

CodiumAI-Agent avatar Aug 15 '24 11:08 CodiumAI-Agent

Oh well, the agent failed to understand a line was added to a function rather than a "duplicate" being added. For a moment I thought it might have caught the duplication between TilesetEditor::editTilesetParameters and TilesetParametersEdit::buttonClicked, but no, I think the latter was likely not in the context.

bjorn avatar Aug 15 '24 11:08 bjorn

Persistent review updated to latest commit https://github.com/mapeditor/tiled/commit/c8a7cdaaa969e62e7b3c514209201a895a9c980e

CodiumAI-Agent avatar Apr 09 '25 14:04 CodiumAI-Agent