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

Assets beginning with a number cause compilation error on windows #1796

Closed
IBwWG opened this issue Mar 31, 2016 · 5 comments
Closed

Assets beginning with a number cause compilation error on windows #1796

IBwWG opened this issue Mar 31, 2016 · 5 comments

Comments

@IBwWG
Copy link
Contributor

IBwWG commented Mar 31, 2016

  • Flixel version: Mar 28 dev
  • OpenFL version: 3.6.0
  • Lime version: 2.9.0
  • Affected targets: windows

Code snippet reproducing the issue:

package;

import flixel.FlxState;

class PlayState extends FlxState
{
    override public function create():Void
    {
        // no code required
    }
}

Name an asset "2p0.wav". The fact that it starts with a number seems to break compilation.


Observed behavior:

include\AssetPaths.h(538): error C2238: unexpected token(s) preceding ';'
include\AssetPaths.h(539): error C2059: syntax error: 'user-defined literal'

Expected behavior: Normal compilation.

@Gama11
Copy link
Member

Gama11 commented Mar 31, 2016

There seem to be two things necessary to reproduce this:

  • don't reference the variable starting with a number directly (for example trace(AssetPaths.0__txt);)
  • add import AssetPaths.hx to make sure the file is still compiled

This is interesting because this should never get through to the C++ compiler, but instead fail during Haxe compilation (which it does, if you reference the variable directly: Unexpected .0).

So there's really two different bugs here, the first one being a Haxe bug (seems like macro-created variables can get around some checks) and the second one an issue with AssetPaths itself (shouldn't create variables with names that are not valid Haxe identifiers). I'm not sure what the best way to deal with that is. Could use escaping of some sort of course (. is already being replaced by __ right now), but if we for example add an underscore to all files starting with a number, that then breaks files that had the underscore there to begin with..

@Gama11 Gama11 added the Bug label Mar 31, 2016
@IBwWG
Copy link
Contributor Author

IBwWG commented Mar 31, 2016

Thanks! I'm glad you teased apart the two bugs!

The issue of flattening filename data with underscores would already be an issue, though...i.e. if you have "file.a.wav" and "file_a.wav" there would be a conflict. And don't other characters get replaced too, like space, parentheses, etc.? In any case it should be possible to check for conflicts, I think...

@Gama11
Copy link
Member

Gama11 commented Mar 31, 2016

The escaping currently looks like this:

value.split("-").join("_").split(".").join("__");

Not very robust.

@Gama11
Copy link
Member

Gama11 commented Mar 31, 2016

Reported the Haxe issue here: HaxeFoundation/haxe#5002

@Gama11
Copy link
Member

Gama11 commented May 25, 2018

For now, we simply ignore assets that would produce invalid identifiers to avoid the compilation errors on native targets. You can still reference those assets using their string path of course, as AssetPaths is just a convenience macro for increased type safety (if you can call it that in this case).

Long-term, we might want a better solution with proper escaping, but perhaps that's not worth the hassle (there's a lot of ways in which a file name could produce an invalid identifer, just think Unicode...). Closing for now.

@Gama11 Gama11 closed this as completed May 25, 2018
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

No branches or pull requests

2 participants