Triplea: Map name and location should not rely on map folder name

[DanVanAtta]

Interesting case in: https://github.com/triplea-game/triplea/issues/1334

A map was downloaded and 'II' was converted to a different character. From then on there were errors loading the map since the map name did not match the map folder.

[RoiEXLab]

Well we could iterate over the files of the map folder and then check if the files are a valid map...

[ron-murhammer]

@DanVanAtta Agree. I think we should be able to just remove the "mapName" property altogether.

[Cernelius]

Please, keep supporting external custom mods not in the repository.

I mean, when you make a mod of a map, and you can play it with whoever has the original map, without him having to download anything else and without you having to put it inside that same map folder (this way you would risk to delete it and, anyway, it would be hard to remember where all your mods are).

I think the objectives.properties stuff should just be all inside the xml.

[simon33-2]

I've got a PR almost ready to go on this but I want to know if you would agree with removing the zip name mangling which causes more than half the errors? Zip should exactly mirror the repo name, perhaps with -master.zip and prevent mismatches between mapName, zip name and repository name.

[veqryn]

How was the map downloaded and why did the name of the file get changed?

That should be figured out first.

The mapName property is required and should continue to be required, please see my comments here: https://github.com/triplea-game/triplea/pull/1651

[simon33-2]

So if we shouldn't remove mapName just make it more forgiving? There are far too many problems with mismatching map name in 1.9. It shouldn't be this hard.

[simon33-2]

Regarding veqryn's comments how was it changed; I think that was largely to support the addition of -master in the filename with the migration to github. I don't really know why that is worth changing though.

[veqryn]

From the other pull request thread:

When maps are uploaded to the new system (github, etc), some other things have to change to them right?
Such as converting attatchment to attachment in the game xml, and maybe some other stuff?

So, why not just correct the contents of the mapname field as part of being uploaded to github?
I feel like that is probably the best long term fix as it is a one time fix per map, keeping everything standardized.

(Fixing the II to turkish ii issue seems like a separate issue and bug that should get fixed in the code that downloads a map.)

[simon33-2]

I think the idea of map name mangling has gone in the wrong direction. What that means is if mapName != mangled mapName, the map fails to start. Can the subdirectory off the root in the zip file which equals mapName be removed completely? It worked that way in 1.8.

[ssoloff]

Can the subdirectory off the root in the zip file which equals mapName be removed completely?

The zipball is generated dynamically by GitHub when requesting the map download URL. GitHub is adding the <repoName>-<branchName> folder at the root of the zipball. I don't think there is any setting in the repo to turn that on or off.

If TripleA initiates the download, I'm sure something could be done transparent to the user whereby the zipball is exploded, all children of the repo-branch folder in the root (not caring what it's named) is moved up to the root, and the zipball is repackaged. However, if the user downloads the zipball outside of TripleA (e.g. using the GitHub Download ZIP link), TripleA would still have to handle the existence of that repo-branch folder.

I had to fix a few bugs recently in the map-making tools that didn't work properly with the new (GitHub) map shape (e.g. the fact that all map resources are now under a folder named map). The approach used in most tools was to try loading files assuming the new shape first and then fall back to the old shape if necessary. Maybe a better approach is, when attempting to load a map from a given folder/zipball, to simply search for a known file (e.g. map.properties) starting from the root. Then consider wherever it's found as the "map path" for file system/classpath operations. Effectively, we would be using a big fat ** wildcard between the root and wherever the map data actually exists.

The only other thing I can think of is to actually have releases in the map repos that contain the map data bundled however we want instead of using the GitHub-managed zipballs. Each map repo already has a Gradle build script, so a new task could be added to perform a release with each Travis build. Obviously, this would require updating the download infrastructure within TripleA itself and the URLs in triplea_maps.yaml. (@RoiEXLab suggested a few months back in #1444 to drop triplea_maps.yaml in favor of using the GitHub API, which might make something like this easier to roll out.)

[veqryn]

My only suggestion is to make things extremely explicit and strict.

Using fuzzy matching, globs, and other hacks is a good way to have a confusing system that is always breaking.

For example, if you have a map who's github address is: github.com/triplea-maps/world_war_ii_v5_1942

If you decide to use github's default zip download, then:
The zip it should download to should be: world_war_ii_v5_1942-master.zip
Inside the zip/folder, the game parser will look for game xml's only under: world_war_ii_v5_1942-master/map/games/*.xml
The the mapname in the xml should be: world_war_ii_v5_1942
The engine will load all map assets (art, polygons, etc) only from world_war_ii_v5_1942-master.zip/world_war_ii_v5_1942-master/map or world_war_ii_v5_1942-master/world_war_ii_v5_1942-master.zip/map (preferring the unzipped folder if both are found, or throwing an error complaining about multiple folders found)

If you decide to go with another method of downloading, then that is cool too, just make it explicit and strict.

[simon33-2]

The example you have put forward there shows mapName == mangledName. There's no issue with that. The issue is when they differ.

[veqryn]

What is mangledName in this context?

So long as any assumptions being made by the engine are explicit, strict, and required, then I would be in favor of the solution.

In my example above, the assumption being made by the engine is that if the mapName = world_war_ii_v5_1942, then the engine will look for a zip file named world_war_ii_v5_1942-master.zip or a folder named world_war_ii_v5_1942-master.
If a folder named world_war_ii_v5_1942 exists, it will be ignored because it doesn't end in -master. If a folder named world_war_ii_v5_1942-mybranch exists, it will be ignored.
The engine will expect that there will then be a folder under this zip/folder that has the exact same name (ie: world_war_ii_v5_1942-master), and that under that will be a folder exactly named map.

This is what I mean by being explicit and strict and required.

[veqryn]

Map "skins" will be a special case, you'll have to decide how those get handled.

(Map skins are map downloads that include only assets but no game xml's, that can be switched to for a different "look and feel" of the current game/map you are playing. For example, I can play an online game of Napoleon, with me looking at the default skin while my opponent uses the "political" skin.)

The old format was to have any folder that matched the mapName but ended in -<something>, be allowed as a map skin.
Example: if your mapName = world_war_ii_v5_1942, then a skin folder/zip would be found under world_war_ii_v5_1942-skin1 or world_war_ii_v5_1942-skin2.zip

Obviously, that won't work with github's system, since github appends -master onto everything when downloaded.

Instead you could have skins be anything mapName-<something>-master, such as: world_war_ii_v5_1942-skin1-master

Or you could switch to something else.

But please consider skins when you formulate the solution to this.

[simon33-2]

Support for this was dropped in 1.9?

mangledName is what is returned by normalizeZipName in ResourceLoader:
protected static String normalizeMapZipName(final String zipName) {
final StringBuilder sb = new StringBuilder();
Character lastChar = null;

final String spacesReplaced = zipName.replace(' ', '_');

for (final char c : spacesReplaced.toCharArray()) {
  // break up camel casing
  if (lastChar != null && Character.isLowerCase(lastChar) && Character.isUpperCase(c)) {
    sb.append("_");
  }
  sb.append(Character.toLowerCase(c));
  lastChar = c;
}
return sb.toString();

}

[Cernelius]

Sorry for reposting the same thing, but I'm going to paste what I've already said:

Please, keep supporting external custom mods not in the repository.

I mean, when you make a mod of a map, and you can play it with whoever has the original map, without him having to download anything else and without you having to put it inside that same map folder (this way you would risk to delete it and, anyway, it would be hard to remember where all your mods are).

Which Veqryn said here more specifically, and I'm just quoting here, to clarify what I meant, with his example:
https://github.com/triplea-game/triplea/pull/1651

I have a folder called world_war_ii_v3 which contains the standard games for v3 (ie: there are several game xml's in it, as well as all the artwork, assets, tiles, etc).
I also have a separate folder called my_v3_mods, which contains some game mods (ie: it just contains the game xml's, and they point to map-name world_war_ii_v3 so that they can pull all the assets from there).

I've the feeling that on the above linked proposal (that @veqryn closed) there is an underestimation of the importance of mods and modding (please don't take this as an accusation to the developers, everyone assigns different importance to each matter, and that is fine, but, right or wrong, this is what I personally see, so I'm saying it, and it is just my opinion; either that, or it is me overestimating it; as I said, it is subjective; I'm not saying this to blame anyone).

@veqryn I've not read in here at all since a while (just seen this because of your name's notification mails), so the lead active developers will tell better then me, I guess, but, for your info, as far as I know, there are not anymore mods "that reference a different folder than the folder they are downloaded in", in the repository. They have all been converted to stand alone (self referencing) maps. Yet, it is still possible having those. What I was talking about was just still supporting the case like the "my_v3_mods", because, if you (the developers) don't, you just make it all the most unfriendly to modders. There has been several issues on the matter, when the decision to not having any external mods in the repository was taken; I guess any developers interested in making the related changes can reference here, if they want to.

For a short summing up of the long story. There were actually proposals to, either, budle all mods in the same main map or restricting to 1 xml per map in any case, and I strongly argued against both, and personally suggested to keep having and supporting external mods. What passed was to convert all collection of external mods, as they were, to stand-alone maps, but the support for external mods was not dropped from the engine (don't!).

Besides the things that are obviously not anymore actual, because of past changes he was not aware of, I 100% agree with all that @veqryn said here and in "Remove Map Name Dependency" (guess I don't have to copy paste and quote everything here, just to say that I agree).
I've repeated my same opinions on this matter several times, in other issues, just like here, this is just a post for confirming it. Sorry for re-posting the same stuff.

[Cernelius]

To be specific, since I'm fearing what I've said might be seen as purely negative, I was referring to:

Support for that would be dropped with this pr. For adding negative value. What is the reason why this is needed and you can't update the map after download?

that is just what I was saying at the first post of this topic (so, I already gave my answer before this question was made); I mean the reasons why it is handy to have, as veqryn said, a "my_v3_mods" folder, and, as I said, I also agree with all the points made by @veqryn, as well, except those things that are not up to date, as he was (is?) not aware that, like, the "Classic_variations" he mentioned, now is simply a stand alone map, with several xml in it (I assume it is; I haven't actually verified the specific case, and guess nothing has changed, lately).

Just totally ignore me here, as I'm not a developer, but I've the personal preference of not to make anything less unforgiving, unless, while working, you also get an error message. But I see that this is a matter of having fewer problems to address; what is the most important, of course, is making the easiest and the fastest for mapmakers to have their work successfully supported.

[DanVanAtta]

There were a number of reasons for doing the merge of base files to map mods. I think when we checked the numbers on how many maps were modded and how - IIRC the majority of mods were full copy/pastes.

I do support allowing for an easy edit of a map xml and then being able to host and play it without others needing to download the full map. There are a number of approaches we can take, and prior constraints when maps were hosted in source forge may no longer apply. For example, it was previously a considerable effort to add files to existing map bundles.

I think we may want a new issue to discuss options for how to best support map modding (we can discuss further here too though).

Revisiting the original problem though, there is a constrain on map names imposed by the name of the map repository, which must also match the map zip name. For example:

- mapName: Big World
  mapCategory: BEST
  url: https://github.com/triplea-maps/big_world/archive/master.zip

In the above mapName and url are actually related, though it is not obvious. There is some freedom in the name 'Big World', but a lower case version of it with spaces replaced with substrings must much the repo name 'big_world'. This is then used to for example find a zip file, eg: big_world-master.zip, but we'll also find folders named that, or zip files big_world. (Yes, we could potentially simplify, lots of backwards compatibility going on)

Summarizing, there are I think two issues at play here:

1. How to best support custom map mods, both informal (IE: I update a file and host), and also more formal (the updated file is uploaded and then available for download via in-game controls).
2. How to allow the mapName text, displayed to users in the download screen, how to allow that text to be arbitrary and not be required to match anything else.

[simon33-2]

If we are still supporting the mapName then why duplicate all the assets folders?

[Cernelius]

I should have said that, of the only 2 Turkish players I'm aware to know, they both had the "ii" converted to dotless "ıı", for folders like "world_war_ii_v3-master", both in the "mapname-master" and in the duplicated "mapname-master" folder inside it, and were unable to play those maps.
This was not happening to them before 1.9, when most names were like "World War II v3", instead (I think all the "ii" were "II", thus already "dotless", from a Turkish perspective, before 1.9).
For info, while in common latin alphabets the upper case "I" is dotless and the lower case "i" is dotted, in Turkey they can both be either dotted or dotless (you can have dotted "I" as well as dotless "i").

So, I suggest to change what at the first post of this topic from:

A map was downloaded and 'II' was converted to a different character.

to

A map was downloaded and "ii" was converted to a different character.

I cannot be sure, but I think the upper case "II" would have been fine.

[DanVanAtta]

Closing for now. We may get a chance to simplify things, for now this topic seems relatively closed.