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

Tile.objectGroup = new ObjectGroup() does not show group in collision editor till different tile is selected #3655

Open
cspotcode opened this issue Apr 13, 2023 · 4 comments
Labels
bug Broken behavior.

Comments

@cspotcode
Copy link

Describe the bug

When a script programmatically adds a new ObjectGroup() to a tile, then adds shapes to that tile.objectGroup, it does not appear in the Collision Editor. It can be made to appear by switching to a different tile, then back to the original tile.

If collision shapes are added in the collision editor before the workaround described above, it seems like the collision editor replaces the scripted objectGroup with a new one.

It seems like the collision editor UI is not notified about the addition of an objectGroup via scripting.

To Reproduce

Steps to reproduce the behavior:

Add the following script to a project:

const setupCollisionActionId = 'setup-collision-action';
const setupCollisionAction = tiled.registerAction(setupCollisionActionId, (a) => {
    const tileset = tiled.activeAsset;
    if(tileset.isTileset) {
        const tileset = tiled.activeAsset;
        const [tile] = tileset.selectedTiles;
        if(tile.objectGroup == null) {
            tile.objectGroup = new ObjectGroup();
        }
        const point = new MapObject('origin');
        point.shape = MapObject.Point;
        tile.objectGroup.addObject(point);
    }
});
setupCollisionAction.text = "Setup Collision";
tiled.extendMenu('TilesetView.Tiles', [{
    action: 'setup-collision-action'
}]);

Open a tileset with at least two tiles, that do not have any collision shapes in the collision editor.
Select the first tile.
Open the collision editor before the next step.
Right-click the tile, click "Setup Collision" to run the scripted action.
Observe that the collision editor does not show the new Point
Click the second tile in the tileset, then click back to the first.
Observe that the collision editor is now showing the new Point

Expected behavior

When scripting adds an objectGroup to a tile, it should appear immediately in the collision editor.

Media

Animation

Specifications:

  • OS: Windows 11
  • Tiled Version: 1.10.1
@cspotcode cspotcode added the bug Broken behavior. label Apr 13, 2023
@eishiya
Copy link
Contributor

eishiya commented Apr 13, 2023

There seem to be a number of assignments you can do that don't trigger a refresh of the relevant displays. Another example is ImageLayer.setImage() (mentioned in a different issue). I wonder if there's some way to discover/fix these en masse.

@cspotcode
Copy link
Author

I dug into the code a bit. Looks like the Collision Editor stores a clone of the tile's objectGroup and synchronizes with the tile every time it changes collision stuff.

ObjectGroup *objectGroup;
if (tile->objectGroup())
objectGroup = tile->objectGroup()->clone();
else
objectGroup = new ObjectGroup;

So I think the Tile needs to emit a signal for this, and the Collision Editor needs to subscribe on that signal and re-clone the objectGroup when it changes.

I'm not sure if we'd hit failures where the user makes changes in the collision editor, but those changes do not immediately get copied to the Tile? If so, the signal would fire and the tile's objectGroup would overwrite the user's modifications in the Collision Editor.

@cspotcode
Copy link
Author

More research:

That event is, indeed, intended for the Collision editor.

/**
* Notifies the TileCollisionDock about the object group of a tile changing.
*/
void tileObjectGroupChanged(Tile *tile);

Emit happens here:

emit tileObjectGroupChanged(tile);
for (MapDocument *mapDocument : mapDocuments())
emit mapDocument->tileObjectGroupChanged(tile);

When scripts assign to objectGroup, I assume they are triggering this method, which does not emit that event:

void EditableTile::setObjectGroup(EditableObjectGroup *editableObjectGroup)
{
if (checkReadOnly())
return;
std::unique_ptr<ObjectGroup> og;
if (editableObjectGroup) {
if (!editableObjectGroup->isOwning()) {
ScriptManager::instance().throwError(QCoreApplication::translate("Script Errors", "ObjectGroup is in use"));
return;
}
og.reset(static_cast<ObjectGroup*>(editableObjectGroup->release()));
}
if (TilesetDocument *doc = tilesetDocument()) {
asset()->push(new ChangeTileObjectGroup(doc, tile(), std::move(og)));
} else {
detachObjectGroup();
tile()->setObjectGroup(std::move(og));
}
if (editableObjectGroup) {
Q_ASSERT(editableObjectGroup->objectGroup() == tile()->objectGroup());
Q_ASSERT(!editableObjectGroup->isOwning());
} else {
Q_ASSERT(tile()->objectGroup() == nullptr);
}
}

@bjorn
Copy link
Member

bjorn commented Apr 15, 2023

When scripts assign to objectGroup, I assume they are triggering this method, which does not emit that event:

When script does this, it should be using ChangeTileObjectGroup, which calls mTilesetDocument->swapTileObjectGroup, which does trigger the event (as per linked code above). So there must be more to this.

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

No branches or pull requests

3 participants