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

Scripting: Enabled creating images and using them #2787

Merged
merged 6 commits into from
Nov 6, 2020
Merged

Conversation

bjorn
Copy link
Member

@bjorn bjorn commented Apr 22, 2020

Now a script can create an image, either directly or by loading from a file or data or based on raw data. Then, the image can be passed to Tileset.loadFromImage or Tile.setImage.

Related to issue #2695

@bjorn
Copy link
Member Author

bjorn commented Apr 22, 2020

@samhocevar I just finished this up and haven't gotten around to testing it nor documenting it, but this should give you quite a few options for getting image data into Tiled and setting it on tilesets or individual tiles, so hopefully it will help with porting your PICO-8 extension to JS.

With this change, images can be dynamically created from JS in three ways:

  • Use image.setPixel(x, y, index_or_rgb) to set each pixel directly.
  • Use new Image(data, width, height, format) to create an image from raw data, see the Format enum for the various supported arrangements.
  • Use image.loadFromData(data, format) to load an image from data in a format like "png" or "bmp".

There are also the methods for modifying the palette.

@bjorn bjorn self-assigned this Aug 7, 2020
@samhocevar
Copy link
Contributor

For future reference, I provided feedback for this feature in #2695 (comment) and resolved current conflicts for this PR in #2906

@bjorn
Copy link
Member Author

bjorn commented Oct 12, 2020

Thanks @samhocevar! Your help is really appreciated and I'll make sure to get around to reviewing this tomorrow.

I'll find out tomorrow but just a quick question, was there anything that stood out while resolving the conflicts? Because I'd prefer to rebase the change instead of merging. Not that it matters much because we can also squash when merging back to master.

@samhocevar
Copy link
Contributor

samhocevar commented Oct 12, 2020

Thanks for the reply! A rebase would be harmless; the conflicts just arose in tiled.pro and tiled.qbs because of the recent addition of scriptfileinfo.cpp which was too close to the scriptimage.cpp entry in the files.

@bjorn bjorn mentioned this pull request Nov 5, 2020
bjorn added 2 commits November 5, 2020 17:09
Now a script can create an image, either directly or by loading from a
file or data or based on raw data. Then, the image can be passed to
Tileset.loadFromImage or Tile.setImage.

Related to issue #2695
@bjorn bjorn force-pushed the wip/scripting-image branch from 79e8e26 to 5221ebc Compare November 5, 2020 16:10
@bjorn
Copy link
Member Author

bjorn commented Nov 5, 2020

  • Image.setColorTable(["#123abc"]) does not work properly, because ScriptImage::setColorTable explicitly casts the array elements to UInt. I have to do Image.setColorTable([0xff123abc]) instead, which is OK but a bit inconsistent with e.g. Tileset.backgroundColor which accepts the "#123abc" argument.

I've fixed this in 9124a7c.

  • calling Tileset.loadFromImage() before Tileset.setTileSize() does not work; the order matters here, otherwise Tiled tries to reload a non-existent image.

Yeah, I'm not quite how to handle this, including how to handle undo when setting the image this way, or saving. I guess in case of saving this could lead to storing the image embedded in the file. I'll need to look into this before it can be merged.

@bjorn
Copy link
Member Author

bjorn commented Nov 6, 2020

@samhocevar For now I just documented that Tileset.loadFromImage needs to be called after Tileset.setTileSize, since it actually cuts up the image so it needs the tile size first. In addition, once the tileset contains tiles but has no "image" set on it, it is assumed to be a collection of images tileset and on such a tileset you're not allowed to set the tile size (because for such a tileset the tile size is automatically set to the largest tile in the set).

If there are any problems with the current API, please let me know!

@bjorn
Copy link
Member Author

bjorn commented Nov 6, 2020

@samhocevar I noticed that you currently have the following code:

https://github.com/samhocevar/tiled-pico8/blob/9eb7529d1495988839989611754b72a3bbddced5/pico8.js#L97-L100

But this is the order which doesn't work, right?

@bjorn bjorn merged commit 8bb8978 into master Nov 6, 2020
@bjorn bjorn deleted the wip/scripting-image branch November 6, 2020 15:14
pancelor added a commit to pancelor/tiled-pico8 that referenced this pull request Feb 19, 2021
* `Image` seems to be new to Tiled 1.5: https://doc.mapeditor.org/en/latest/reference/scripting/#image
* fix some out of order commands, mentioned here: mapeditor/tiled#2787 (comment)
* The thing with #123abc seems to work now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants