lafriks / go-tiled

Go library to parse Tiled map editor file format (TMX) and render map to image
MIT License
204 stars 42 forks source link

Parsing error due to incorrect slashes with embed on windows. #75

Closed sedyh closed 1 year ago

sedyh commented 1 year ago

Problem

Hello, I found a problem in the operation of one of the UnmarshalXML in Map. This code will cause os.ErrNotExist on windows for data/world/surface-tileset.tmx even if the file is there:

//go:embed data
var fs embed.FS
...
// File surface.tmx has reference to surface-tileset.tmx in it.
tiled.LoadFile("data/world/surface.tmx", tiled.WithFileSystem(fs))

The library allows you to insert your own fs implementation to upload files: https://github.com/lafriks/go-tiled/blob/main/tiled.go#L76

In the process, it loads additional files (for example, tileset) for which it builds a path as follows: https://github.com/lafriks/go-tiled/blob/main/tmx_map.go#L173

Which will return the path with windows-like slashes data\world\surface-tileset.tmx. This path will not work with embed: https://github.com/golang/go/issues/44305

Despite the fact that these are details of the implementation of a specific fs, it is part of the standard library. So it makes sense to take these details into account inside the lib.

Solution A

Add a wrapper that converts paths in special cases.

func WithFileSystem(fileSystem fs.FS) LoaderOption {
    return func(l *loader) {
        l.FileSystem = fileSystem
        if _, ok := l.FileSystem.(*embed.FS); ok {
            l.FileSystem = WithPathRelative(FileSystem)
        }
    }
}

type PathRelativeFS struct {
    fs fs.FS
}

func WithPathRelative(fs fs.FS) fs.FS {
    return &PathRelativeFS{fs}
}

func (p *PathRelativeFS) Open(name string) (fs.File, error) {
    name = strings.Replace(name, string(filepath.Separator), "/", -1)
    return p.fs.Open(name)
}

Solution B

Add the option to use path.Join instead of filepath.Join for special occasions.

func (m *Map) GetFileFullPath(fileName string) string {
    if _, ok := m.loader.FileSystem.(*embed.FS); ok {
        return path.Join(m.baseDir, fileName)
    }
    return filepath.Join(m.baseDir, fileName)
}

Looks like it the same as #63.

lafriks commented 1 year ago

Looks like duplicate for #63 closing this.

There is also provided better solution for this: https://github.com/tslocum/go-tiled/commit/2d2dec486bd90b8c2b88c37a81615cc0e0ac4728

imho it is not correct to use path.Join

lafriks commented 1 year ago

feel free to send PR for this

sedyh commented 1 year ago

Thats strange... no one has made a fix for the issue since last year. I'l try, if I have time.

lafriks commented 1 year ago

I have no windows to test on, so I not really want to implement something I'm not able to even test :]