Skip to content

Fix crash when saving tilemap's cel image #4089

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.

Sign up for GitHub

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

Jump to bottom
Merged
dacap merged 1 commit into from
Nov 2, 2023
Merged

Fix crash when saving tilemap's cel image #4089

dacap merged 1 commit into from
Nov 2, 2023

Conversation

@martincapello
Copy link
Member

@martincapello martincapello commented Oct 6, 2023

After applying this PR:

  1. A script can save a cel's image from a tilemap layer without crashing Aseprite.
  2. The saved image is the rendered image using the tileset tiles for each tilemap index.
  3. A new lua test is added.

Fix #4069

@martincapello martincapello requested a review from dacap as a code owner October 6, 2023 18:20
@martincapello martincapello force-pushed the fix-save-tilemap-cel-image branch 2 times, most recently from 16bf6b5 to dae68a1 Compare October 6, 2023 19:06
@dacap
Copy link
Member

dacap commented Oct 11, 2023

Didn't review this, waiting for @Gasparoken, anyway it would be nice to add a note about any new sprite added in the tests/sprites/README.md file.

@Gasparoken
Copy link
Member

Gasparoken commented Oct 12, 2023

I'll review this today. Sorry for the delay.

@martincapello martincapello force-pushed the fix-save-tilemap-cel-image branch 2 times, most recently from 54da35a to 40f974b Compare October 12, 2023 16:51
@Gasparoken
Copy link
Member

Gasparoken commented Oct 12, 2023

Works great! I think the name MakeStdTilemap could be improved to describe:
"Make a blank sprite with Tilemap layer and specific Tileset"

@martincapello
Copy link
Member Author

martincapello commented Oct 17, 2023 •
edited
Loading

Would MakeStdTilemapSpriteWithTileset work?

@Gasparoken
Copy link
Member

Gasparoken commented Oct 17, 2023

Would MakeStdTilemapSpriteWithTileset work?

LGTM!😊

@Gasparoken
Copy link
Member

Gasparoken commented Oct 17, 2023

I was testing and found that this solution saves an image with the size of the sprite (which could be different than the size of the image in the cel) and the image is placed at origin = 0.0. Therefore, it is necessary to shrink the image before saving, or define the correct size of the final image from the beginning.

@martincapello martincapello force-pushed the fix-save-tilemap-cel-image branch from 40f974b to b5cec7b Compare October 17, 2023 19:40
@martincapello martincapello force-pushed the fix-save-tilemap-cel-image branch from b5cec7b to c700709 Compare October 17, 2023 19:50
Gasparoken
Gasparoken approved these changes Oct 18, 2023
View reviewed changes
Copy link
Member

@Gasparoken Gasparoken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🙌

@Gasparoken Gasparoken assigned dacap and unassigned Gasparoken Oct 18, 2023
@dacap dacap merged commit 3cfa5ef into aseprite:main Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Gasparoken Gasparoken Gasparoken approved these changes

@dacap dacap Awaiting requested review from dacap dacap is a code owner

Assignees

@dacap dacap

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tilemap cel image saveAs call leads to segmentation fault.

3 participants

@martincapello @dacap @Gasparoken