json support #18

synchronisator · 2021-01-23T19:26:02Z

This PR enables json support for tiled maps.
The compare-test shows, that the test.tmx and the same map as test.json have the same output.
A user jan load a json, but decide to use the tmx strusture instead by converting it.

lib/src/object_group.dart

lib/src/tile_map_parser.dart

json_structure.puml

wolfenrain

Did a quick review, few comments.

lib/src/object_group.dart

lib/src/tmx_object.dart

json_structure.puml

lib/src/layer.dart

lib/src/tile_map_json_parser.dart

luanpotter · 2021-02-16T21:46:13Z

Sorry for the long delay @synchronisator I had some harsh weeks at work. But lets get back to this as I think json support is very useful. I will re-open and address any remaining comments myself.

synchronisator · 2021-02-16T22:12:14Z

Sorry for the long delay @synchronisator I had some harsh weeks at work. But lets get back to this as I think json support is very useful. I will re-open and address any remaining comments myself.

I have rewritten everything, but have not yet made a new pr. Look here: https://github.com/synchronisator/tiled.dart/tree/jsonsupport

luanpotter · 2021-02-17T04:16:00Z

@synchronisator ah sounds good! I will wait for your PR then. lmk if I can help with anything :)

synchronisator · 2021-03-04T10:56:35Z

Hi,
sorry for the long time not changing anything. Had much work here...
Now i try to answer all of your questions:

"computeDrawRect": "can we just move it up the data hierarchy? if not ok to put on flame_tiled"

We could add this to "tileset" like this:

  Rectangle computeDrawRect(Tile tile) {
    if (tile.image != null) {
      return Rectangle(0, 0, tile.image.width, tile.image.height);
    }
    final row = (tile.gid - firstGId) ~/ columns;
    final column = (tile.gid - firstGId) % columns;
    final x = margin + (column * (tileWidth + spacing));
    final y = margin + (row * (tileHeight + spacing));
    return Rectangle(x, y, tileWidth + spacing, tileHeight + spacing);
  }

Is this a solution you would like?

synchronisator · 2021-03-04T11:08:52Z

"make sure we have fallbacks so the old TMX files still work"

The old test.tmx failes with the current version of my code. This is because the tilecount is missing in the file. But the tilecount is important to create the tileMatrix.
The old Code is buggy at this point. It uses the tileWidth and tileHeight

      width = dsl.intOr('tilewidth', width);
      height = dsl.intOr('tileheight', height);

But this is wrong.

I could recreate this buggy behavior if the tileCount is not available but i dont think this is a good solution....

Edit: Hm...something is not right here... i will have a look at my tileset code again. no time now, but the code where i use tilecount is not for TileMatrix (thats in layer) but for tiles-List. I will have a deeper look later.

Edit2: ok, i got it. Sorry i was a little bit confused.
The Code

    final iterator = tilelist.iterator;
    Tile t = iterator.moveNext() ? iterator.current : Tile(-1);
    for (var i = 0; i < tileCount; ++i) {
      if(t.gid == i){
        tiles.add(t);
        if(iterator.moveNext()) {
          t = iterator.current;
        }
      }else{
        tiles.add(Tile(i));
      }
    }

is needed to add every "empty" tile to the tiles.
The file only contains the tilecount and every "not empty" tile. So it has to be filled up here.
But for that i need the tilecount.

SOLUTION:
If there is not tileCount i will use the last found GId to fill empty tiles up:

    if(tileCount == null && tilelist.isNotEmpty){
      tileCount = tilelist.last.gid;
    }

That is a hack for very old maps (older than 5 years)
Additionaly i think i found a bug in my code because my loop only go < tileCount. But the tiles include the empty 0 tile without counting it in tileCount so i had to go until <= tileCount.
I changed this. please have a look

synchronisator · 2021-03-04T18:33:07Z

So. we reach the last three questions:

uncommented Code:
i will fix this ... soon.
make sure we keep an example for the old TMX format not throwing errors
I added it again as test_old.tmx
you added a few images, is that ok in terms of licensing? ie are they all open source?
I have the maps from three other repos on github:
floortileset.png and tilesets_deviant_milkian_1.png are from Workadventu.re
https://github.com/thecodingmachine/workadventure/tree/develop/maps/Lyon

isometric_grass_and_water.png is from Tiled
https://github.com/mapeditor/tiled/tree/55175d1930b98087672a2f83ac9f30ddb8fed2c4/examples

And coins.png is from flutter_tiled
https://github.com/flame-engine/flame_tiled/tree/master/example/assets/images

If the images are ok, the only open task would be to make the tests clean and without uncommented code. I will work on that in the next time.
If you have more comments please let me know.

synchronisator · 2021-03-18T15:48:01Z

Hi, i updated the tests.
There are 2 problems left.

The computeDrawRect is moved to TileSet, but return "other" results and i have no clue why.
In tmx_object_test.dart there is a test for tiledObject.isRectangle but there is no value inside the files to see if its a rectangle, and i dont know how to compute this from the polygon.

Maybe you have an idea.
Thanks

Akida31 · 2021-03-29T20:52:46Z

@synchronisator after testing with the TiledMapEditor, rectangles have no certain property set and no polygon/ polyline but only width and height. The width and height could be zero, that would mean that the rectangle has the size of one tile (from what i can see).
Probably it would be good to add the getter tiledObject.isRectangle so that not only the tests but also the users could use this.

The question is if isRectangle should check the polygon if it has 4 points creating a rectangle (which could be done be some math)

synchronisator · 2021-03-30T06:30:33Z

@synchronisator after testing with the TiledMapEditor, rectangles have no certain property set and no polygon/ polyline but only width and height. The width and height could be zero, that would mean that the rectangle has the size of one tile (from what i can see).
Probably it would be good to add the getter tiledObject.isRectangle so that not only the tests but also the users could use this.

The question is if isRectangle should check the polygon if it has 4 points creating a rectangle (which could be done be some math)

@Akida31
Yes, this is exactly the thing i wrote as my second problem. I dont know how to do this check correct. From where do you have the information about "empty width/height means tilewidth/tileheight"?
Yes, math would do the "4points creating a rectangle"-job. I was not sure if this is the way i should do that. But if the others think the same way, i will calculate if its a rectangle.

Akida31 · 2021-03-30T09:11:16Z

I just created a map with different shapes and looked into the Json-Export-file. Probably not the cleanest way. Would be nice if you could verify that my assumptions are true.
It was an addition to your second question and I thought it would help you to hear an opinion/ some information related to the problem. If you want I could write the implementation the way I would solve that

synchronisator · 2021-03-30T12:31:33Z

@Akida31 Thanks for your testing.
I looks like you are right. If i dont "drag a size" to a rectangle in Tiled it has a default width/height but its not tileheight/tilewidth. In my testmap i have 32x32 Tiles. But the created rectangle is 20x20. its strange.

Something similar is happening with an ellipse without dragging. here is the height 20 again without a height in the json. Maybe its a tiled default.

After a little bit of analysing. I think the rule is: If its NOT a polygon or polyline or ellipse or point it IS a rectangle.
This includes "rotated" rectangles without math calculations.

Question to all of you: Is it needed to calculate rectangles from polygons?

synchronisator · 2021-03-30T13:06:21Z

I asked some people in the tiled discord and learned two things:

This is correct: If its NOT a polygon or polyline or ellipse or point it IS a rectangle.
the "defaultsize 20" is a hack in tiled to "show an object with 0 size". So i would let the size empty and will not set 20.

spydon · 2021-04-21T07:43:10Z

@synchronisator great work! How is it going, anything you need help with? :)

Once this is in we will start migrating this package to null-safety.

synchronisator · 2021-04-21T08:03:30Z

@spydon thanks
The last thing is (I quote myself)
"The computeDrawRect is moved to TileSet, but return "other" results and i have no clue why."

If someone has any ideas on this, please comment. I have no idea what is going wrong here...

spydon · 2021-04-21T08:24:21Z

@spydon thanks
The last thing is (I quote myself)
"The computeDrawRect is moved to TileSet, but return "other" results and i have no clue why."

If someone has any ideas on this, please comment. I have no idea what is going wrong here...

Thanks for the quick reply! I'll have a look with the team.
What does it mean with "other" results btw, do you have an example?

synchronisator · 2021-04-21T08:32:55Z

I am not at my pc right now. But I think there was a test for this method that fails now... But I am not 100% sure. Will have a look later

synchronisator · 2021-04-21T09:12:04Z

@spydon
It is part of the parser_test.dart line 158 following
i uncommented it.

Other things:
There are 4 other TODOs in the code. Maybe someone has things to say to clear them out.

tile_test.dart: // TODO no default constructor on Tileset
so the test is diffrent than before, but i think that is ok.
layer.dart: //TODO zstd compression not supported in dart
There is no zstd compression in dart so i think this is ok as well
template.dart: // TODO is it possible to have an externel tsx here?
I have no idea if this is possible, but i dont think so...
tile.dart: //TODO correct?
A Question about json parsing and if the default is correct. This could be good to be reviewed by someone.

luanpotter · 2021-04-21T16:05:54Z

Haven't dug deep into what each one is, but my initial 2cents:

// TODO no default constructor on Tileset
can't we just add a default constructor? no issues for me if it's needed

//TODO zstd compression not supported in dart
just leave out this feature for now

// TODO is it possible to have an externel tsx here?
just leave out this option for now

//TODO correct?
that I will take a look into the code to see what it's about haha

luanpotter · 2021-04-21T16:06:24Z

Since this PR is so useful and so big, I think we can be a bit lenient with missing features and followups, as long as they are well documented here :)

synchronisator · 2021-05-12T12:56:57Z

Hi,
this PR is waiting now for a long time.
What do i have to change? What do you expect happens next?
Who would approve/merge this?
Thanks

spydon · 2021-05-12T13:46:44Z

test/rectangle_map_test.dart

+      expect(
+          layer.tileIDMatrix[8], equals([0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0]));
+      expect(
+          layer.tileIDMatrix[9], equals([0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0]));


Minor comment, these should have a trailing comma since it is split on two lines.

we can do that as a followup

luanpotter

let's merge this and address things as needed

synchronisator added 3 commits January 23, 2021 20:22

json support

f0452fe

jsonsupport irrelevant test deleted; readme changed

52aa959

style

fcdc0f5

luanpotter reviewed Jan 23, 2021

View reviewed changes

lib/src/object_group.dart Outdated Show resolved Hide resolved

luanpotter reviewed Jan 23, 2021

View reviewed changes

lib/src/tile_map_parser.dart Outdated Show resolved Hide resolved

luanpotter reviewed Jan 23, 2021

View reviewed changes

json_structure.puml Outdated Show resolved Hide resolved

comments

874af96

wolfenrain reviewed Jan 24, 2021

View reviewed changes

lib/src/object_group.dart Outdated Show resolved Hide resolved

lib/src/tmx_object.dart Outdated Show resolved Hide resolved

json_structure.puml Outdated Show resolved Hide resolved

lib/src/layer.dart Outdated Show resolved Hide resolved

comments

3cf79e8

synchronisator commented Jan 24, 2021

View reviewed changes

lib/src/tile_map_json_parser.dart Outdated Show resolved Hide resolved

synchronisator closed this Jan 25, 2021

synchronisator deleted the jsonsupport branch January 25, 2021 08:10

synchronisator restored the jsonsupport branch January 25, 2021 08:13

synchronisator added 13 commits January 25, 2021 09:52

new directory

ea1f405

Tileset fromXml

24d1b0a

xml parse

2974c66

analyzer

d2cb1f0

tmx support updated

c27e2da

template added

e049724

objectgrouptest

fe0e8f4

layer_test

a286b4c

tests

ce89818

prevent empty input

3e02c3c

animation added to complexmap

0059df4

chunks and infinite

ee93526

layer group

858de9c

luanpotter reopened this Feb 16, 2021

synchronisator added 4 commits February 24, 2021 09:42

map

95fd243

enums

6f28c9b

bugfix generateTiles

6f2f786

double Point

fe49280

tileCount missing in old maps

f362316

Tests

fb96303

isRectangle; Tileset defaultvalues

dcfc60e

synchronisator added 2 commits April 21, 2021 12:17

lint

1fab1de

remove unused

db2de4d

spydon reviewed May 12, 2021

View reviewed changes

spydon approved these changes May 12, 2021

View reviewed changes

luanpotter approved these changes May 12, 2021

View reviewed changes

luanpotter merged commit 14a87cd into flame-engine:main May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

json support #18

json support #18

synchronisator commented Jan 23, 2021

wolfenrain left a comment

luanpotter commented Feb 16, 2021

synchronisator commented Feb 16, 2021

luanpotter commented Feb 17, 2021

synchronisator commented Mar 4, 2021 •

edited

synchronisator commented Mar 4, 2021 •

edited

synchronisator commented Mar 4, 2021

synchronisator commented Mar 18, 2021 •

edited

Akida31 commented Mar 29, 2021

synchronisator commented Mar 30, 2021

Akida31 commented Mar 30, 2021

synchronisator commented Mar 30, 2021

synchronisator commented Mar 30, 2021

spydon commented Apr 21, 2021

synchronisator commented Apr 21, 2021

spydon commented Apr 21, 2021

synchronisator commented Apr 21, 2021

synchronisator commented Apr 21, 2021

luanpotter commented Apr 21, 2021

luanpotter commented Apr 21, 2021

synchronisator commented May 12, 2021

spydon May 12, 2021

luanpotter May 12, 2021

luanpotter left a comment

json support #18

json support #18

Conversation

synchronisator commented Jan 23, 2021

wolfenrain left a comment

Choose a reason for hiding this comment

luanpotter commented Feb 16, 2021

synchronisator commented Feb 16, 2021

luanpotter commented Feb 17, 2021

synchronisator commented Mar 4, 2021 • edited

synchronisator commented Mar 4, 2021 • edited

synchronisator commented Mar 4, 2021

synchronisator commented Mar 18, 2021 • edited

Akida31 commented Mar 29, 2021

synchronisator commented Mar 30, 2021

Akida31 commented Mar 30, 2021

synchronisator commented Mar 30, 2021

synchronisator commented Mar 30, 2021

spydon commented Apr 21, 2021

synchronisator commented Apr 21, 2021

spydon commented Apr 21, 2021

synchronisator commented Apr 21, 2021

synchronisator commented Apr 21, 2021

luanpotter commented Apr 21, 2021

luanpotter commented Apr 21, 2021

synchronisator commented May 12, 2021

spydon May 12, 2021

Choose a reason for hiding this comment

luanpotter May 12, 2021

Choose a reason for hiding this comment

luanpotter left a comment

Choose a reason for hiding this comment

synchronisator commented Mar 4, 2021 •

edited

synchronisator commented Mar 4, 2021 •

edited

synchronisator commented Mar 18, 2021 •

edited