libtiled: tileset source should be read relative to the tmx, not current directory

[hoxu]

TMXMapReader.unmarshalTileset calls TMXMapReader.makeUrl to convert a tileset source path to a URL:

https://github.com/bjorn/tiled/blob/3fee6ebfa08a949c30e1324ca6a3da0765309524/util/java/libtiled-java/src/main/java/org/mapeditor/io/TMXMapReader.java#L114-L120

The path of the tmx being loaded is not used, so the tileset source is assumed to be in the current directory. This is an invalid assumption, as tiled itself saves a path relative to the tmx.

Relevant part of stack trace:

	at org.mapeditor.io.TMXMapReader.unmarshalTileset(TMXMapReader.java:241)
	at org.mapeditor.io.TMXMapReader.buildMap(TMXMapReader.java:738)
	at org.mapeditor.io.TMXMapReader.unmarshal(TMXMapReader.java:786)
	at org.mapeditor.io.TMXMapReader.readMap(TMXMapReader.java:833)

TMXMapReader.unmarshalTileset should construct a path from the tmx file's directory and the (relative) <tileset source="...">.

[hoxu]

This seems to be working fine with the TMXMapReader.readMap(URL) variant added in #2829.

Which unfortunately is not in the latest maven artifact (1.2.3). I'll close this issue, and create a new one about the maven artifact being outdated.

[bjorn]

@hoxu But this issue is still relevant for the String version of readMap, right? If so I think we should leave this open.

[hoxu]

No. I created this issue too hastily and with poor details, apologies. I tested this again with both the latest maven artifact and manually built snapshot, and readMap(String) works.

readMap(InputStream) is the problematic one, and naturally it doesn't know of the path of the passed InputStream. I probably overlooked that because I had some abstraction code in the middle for mapping filenames to resource streams, before I realized the master branch has readMap(URL).

The problem is in the xmlPath field assignment, that uses user.dir:

https://github.com/bjorn/tiled/blob/8bb8978a5191c4fa9a61a2643d6fac5eea33ec55/util/java/libtiled-java/src/main/java/org/mapeditor/io/TMXMapReader.java#L921-L924

I think it would make sense to either 1) add a path parameter that is used instead of user.home, or 2) deprecate this method in favor of readMap(URL).

[bjorn]

@hoxu Alright, thanks for the additional info! I think I would go for option 1 since I guess there is still some extra flexibility compared to requiring a URL. Is it something you would like to patch up?

[hoxu]

@bjorn see #2932.