feat: Export maps #78

benni-tec · 2024-01-16T22:07:28Z

Description

This adds the ability to export maps as well as import them into json and tmx formats

Checklist

The title of my PR starts with a [Conventional Commit] prefix (fix:, feat:, docs: etc).
I have read the [Contributor Guide] and followed the process outlined for submitting PRs.
--> melos run analyze returns a lot of fatal infos, however they do not orginate in this PR
I have updated/added tests for ALL new/updated/fixed functionality.
--> Not yet. There are tests for encoding tile layers and some basic checks for maps, I can write more extensive tests if needed, however this will take same time as my exams are comming up shortly!
I have updated/added relevant documentation in docs and added dartdoc comments with ///.
--> I would gladly provide some more documentation however I did not find a place for it, similarly there does not seem to be any documentation for the parser
I have updated/added relevant examples in examples.
--> I will get to this soon, but would appreciate some guidance on my other questions and code?

Breaking Change

Yes, this is a breaking change.
No, this is not a breaking change.

Related Issues

Map Export #71

I have taken the liberty to rebase this branch to the current state of #77 as to integrate the new types introduced there!

--> replaced dart:ui Color with color_model package --> custom Rect instead of dart:ui --> replaced flutter_test with test package

…or.dart

…/ added tests for hex decoding in ColorData

…ter type safety)

…ageRect

jtmcdole

This is a huge CL and I'm only half way through with it. I'll probably want more eyes on it than just mine.

Question: what's the motivation for exporting Tiled data? Are you building a Tiled Map Editor in Flutter/Dart? Is it for saving modified maps in game?

jtmcdole · 2024-01-17T14:49:16Z

packages/tiled/lib/src/common/color.dart

+/// Basic data class holding a Color in ARGB format.
+/// This can be converted to dart:ui's Color using the flame_tiled package
+class ColorData extends ExportValue<String> {
+  static int _sub(int hex, int index) => (hex >> index * 8) & 0x000000ff;


Why not store the hex value in ColorData and have getters that returns the bit-shifted values back when needed? export then becomes => _hex.toRadixString(16).padLeft(8, '0'). My assumption right now is this will increase the size of memory.

Yes you are right that would be more efficient!

jtmcdole · 2024-01-17T14:55:39Z

packages/tiled/lib/src/common/iterable.dart

+    final out = <K, List<V>>{};
+    for (final v in this) {
+      final k = key(v);
+      if (!out.containsKey(k)) {


Pick your poison; but since you have "List" as non-nullable, the first is shorter.

(foo[1] ??= []).add('one'); foo.putIfAbsent(2, ()=>[]).add('two');

jtmcdole · 2024-01-17T14:58:36Z

packages/tiled/lib/src/exporter/export_element.dart

+  final Map<String, ExportObject> children;
+  final CustomProperties? properties;
+
+  const ExportElement(


I'd like for these to be named parameters. It would make the earlier calls to ExportElement easier to read, and bonus, you could default values (e.g. chilren = {}) and make the call sites look cleaner.

jtmcdole · 2024-01-17T15:01:56Z

packages/tiled/lib/src/exporter/export_value.dart

+  _ExportablePointList(this.points);
+
+  @override
+  List<Map<String, double>> get json => points


[for (final point in points) { 'x': point.x, 'y': point.y}];

jtmcdole · 2024-01-17T15:05:40Z

packages/tiled/lib/src/layer.dart

+  ExportElement export() => ExportElement(
+        'objectgroup',
+        {
+          'id': id?.toExport(),


You're ok with this being "id": null? Same for class below?

No not really, but as it stands it is nullable in the class. So if it is allowed to import with a null value it think it should be possible to export with a null value

Yeah, so I was wondering if I don't have an ID in my tiled file and then I export, I get a null id; will that break someone else. If its not present, we could just add it to the map as a map-if:
{
if (id != null) 'id': id!.toExport(),
}

same for other nullables.

benni-tec · 2024-01-17T18:30:33Z

This is a huge CL and I'm only half way through with it. I'll probably want more eyes on it than just mine.

Sorry, this is my first real PR. What does CL mean? 😅

Question: what's the motivation for exporting Tiled data? Are you building a Tiled Map Editor in Flutter/Dart? Is it for saving modified maps in game?

Well I wanted to build a game with flame_tiled however the tilesets I'm using are HUGE but I only use a small subset of them, so I wrote an optimizer in dart. In hindsight it would have been smarter to use another language, altough I could not find a tmx-library that can export apart from python (which is horribly inefficient) and C++ (which I'm not proficient in) I think!

jtmcdole · 2024-01-18T00:04:17Z

Sorry, this is my first real PR. What does CL mean? 😅

ah, sorry, "change list"; its the same as PR / Pull Request!

jtmcdole · 2024-01-18T00:05:27Z

Well I wanted to build a game with flame_tiled however the tilesets I'm using are HUGE but I only use a small subset of them, so I wrote an optimizer in dart. In hindsight it would have been smarter to use another language, altough I could not find a tmx-library that can export apart from python (which is horribly inefficient) and C++ (which I'm not proficient in) I think!

Could these be extensions on top of the classes that would then be optional for anyone else? It sounds like a neat project.

benni-tec · 2024-01-18T08:09:50Z

Could these be extensions on top of the classes that would then be optional for anyone else?

I don‘t think it is possible to achieve this using extensions, since they can not implement classes/interfaces and therefore it would be impossible to build an export tree.

Using extensions could work if I recreate all classes, use extensions to convert to them (this would require a huge amount of boilerplate code) and then build a separate hierarchy. However that would require a lot of effort to implement, double the required space while exporting and make it hard to maintain (keeping track if changes in this repo)!

I don‘t really see why this couldn’t just stay in the main package, is there any particular reason?

jtmcdole · 2024-01-19T01:31:52Z

I don‘t really see why this couldn’t just stay in the main package, is there any particular reason?

Maintenance.

You could totally do mixins since you're not really adding any values to the classes that are there solely for rendering.

extension TileLayerExporter on TileLayer {
 // all things exportable
}

extension ChunkExporter on Chunk {
ExportElement export() {... code}
}

There's no boilerplate; it just keeps the base classes cleaner for the 99% case of consumption.

side note; I think you mean implements Exportable instead of with. I thought "with" was for mixins. I guess there's not actually a compile here / warning here, so that's neat.

benni-tec · 2024-01-19T15:20:55Z

side note; I think you mean implements Exportable instead of with. I thought "with" was for mixins. I guess there's not actually a compile here / warning here, so that's neat.

Actually no, I'm using the Exportable abstract class as a mixin, this way the mixin implements ExportResolver, with it's exportXML() and exportJson() which is needed for the export tree by delegating to the ExportResolver that is provided to the export() method of Exportable.

This way I can just pass the object into the export tree insted of having to call x.export() every time.

abstract class Exportable implements ExportResolver {
  /// Returns the proxy usually an [ExportElement]
  ExportResolver export();

  @override
  XmlNode exportXml() => export().exportXml();

  @override
  JsonObject exportJson() => export().exportJson();
}

There's no boilerplate; it just keeps the base classes cleaner for the 99% case of consumption.

That's also where extra boilerplate code would come from when doing extensions, so I would prefer it to be included here. However if you do not want to maintain it, I can create an extension package for it. Altough I don't think there is actually going to be much maintenance since (as far as I unterstand) the tmx standard is actually pretty old and does not change anymore!

spydon · 2024-01-21T18:43:08Z

the tmx standard is actually pretty old and does not change anymore!

It is pretty old, but it does still update:
https://doc.mapeditor.org/en/stable/reference/tmx-changelog/

benni-tec added 17 commits October 24, 2023 23:55

removed need for flutter sdk (dart:ui)

c37b7cf

--> replaced dart:ui Color with color_model package --> custom Rect instead of dart:ui --> replaced flutter_test with test package

replaced custom rect in favor of Rectangle from dart:math

e389a08

removed color_models dependency in favor of a basic data class in col…

8f38c1c

…or.dart

renamed Color to ColorData to make conversion easier in flame_tiled /…

f697800

…/ added tests for hex decoding in ColorData

added dartdocs to color and tests

e104efd

added back flame_lint

6b60e92

transfer

54c3878

added exports to I think everything

50025a0

added some tests and fixes

ef1e6ab

removed ExportSettings and changed JsonExport to use JsonObjects (bet…

e1c4521

…ter type safety)

rebase onto flame-engine#69-dart-sdk

9174a0a

fixed typo

a7d451c

added imageRect passthrough for explicit tiles without an existent im…

657a547

…ageRect

fixed properties missing on layers

54f4a06

fixed base64 encoding and added tests for ExportTileData

c0452f8

formatted

4652080

formatted, nicer structure and dartdocs

3a057e0

spydon requested review from kurtome, jtmcdole, ufrshubham and erickzanardo January 17, 2024 07:32

spydon changed the title ~~feat: Export~~ feat: Export maps Jan 17, 2024

jtmcdole reviewed Jan 17, 2024

View reviewed changes

benni-tec mentioned this pull request Jan 18, 2024

Dart SDK compatibility #69

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Export maps #78

feat: Export maps #78

benni-tec commented Jan 16, 2024 •

edited

Loading

jtmcdole left a comment

jtmcdole Jan 17, 2024

benni-tec Jan 17, 2024

jtmcdole Jan 17, 2024

jtmcdole Jan 17, 2024

jtmcdole Jan 17, 2024

jtmcdole Jan 17, 2024

benni-tec Jan 17, 2024

jtmcdole Jan 18, 2024

benni-tec commented Jan 17, 2024 •

edited

Loading

jtmcdole commented Jan 18, 2024

jtmcdole commented Jan 18, 2024

benni-tec commented Jan 18, 2024 •

edited

Loading

jtmcdole commented Jan 19, 2024

benni-tec commented Jan 19, 2024 •

edited

Loading

spydon commented Jan 21, 2024

feat: Export maps #78

Are you sure you want to change the base?

feat: Export maps #78

Conversation

benni-tec commented Jan 16, 2024 • edited Loading

Description

Checklist

Breaking Change

Related Issues

jtmcdole left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benni-tec commented Jan 17, 2024 • edited Loading

jtmcdole commented Jan 18, 2024

jtmcdole commented Jan 18, 2024

benni-tec commented Jan 18, 2024 • edited Loading

jtmcdole commented Jan 19, 2024

benni-tec commented Jan 19, 2024 • edited Loading

spydon commented Jan 21, 2024

benni-tec commented Jan 16, 2024 •

edited

Loading

benni-tec commented Jan 17, 2024 •

edited

Loading

benni-tec commented Jan 18, 2024 •

edited

Loading

benni-tec commented Jan 19, 2024 •

edited

Loading