Loading Unzipped Map From In-Game Download

[Cernel]

@prastle I'd just wait what a developer have to say, as I can't believe this thing of needing different paths zipped vs unzipped may be intended; at most accepted (the fact that you need not having the "-master" in the name when unzipped).
Probably here we need @LaFayette , if he will read this post, since the new "master" thing was his creation.

[General_Zod]

This didn't change when "-master" was added. It happened somewhere afterwards for no apparent reason. Because I recall not requiring to zip files even with that suffix.

Another point to add is that it used to always read and loaded the unzipped version before the zipped version if both were present. However this only applied to the xml itself, the rest of the files still had to be zipped.

It would be best if it read/loaded all the files (not just xml) from a unzipped folder always. This would save a lot of time when mapmaking and testing small changes. ( in 1.8 days it might have done so, don't recall)

[redrum]

So I think this is probably a side effect of all the map rework. Probably would be best to allow unzipped maps to have *-master so its easier to unzip downloaded map and tweak them.

[Cernel]

@redrum said in Open unzipped map:

So I think this is probably a side effect of all the map rework. Probably would be best to allow unzipped maps to have *-master so its easier to unzip downloaded map and tweak them.

Definitely. This thing of having different names zipped and unzipped doesn't make sense at all.

[prastle]

@redrum it occurs for the pulls from the downloader and the bots
they both pull from the master only. You can have 20 versions of a map in the repo. The updater only pulls the master. I know this from updating the bots.

[Cernel]

@redrum I always wondered if this "-master" addition would create any conflicts with the fact that "-" also identifies mapskins. So maybe keep an eye open on that (might be not relevant, idk).

[prastle]

@redrum yes if all had master would be easier just make sure all unzipped have master added now If ya want to go that route every map would have to be rewritten. it is much easier to remove the master. Since it is only needed for maps to be updated to the downloader or bots not every map we have ever created.

[redrum]

PR: https://github.com/triplea-game/triplea/pull/3339

Map name is always found from the XML property
<property name="mapName" value="total_world_war" editable="false">
<string/>
</property>

Unzipped Map Directory Examples (in downloadedMaps)

1. total_world_war
2. total_world_war/map
3. total_world_war-master
4. total_world_war-master/map

Zipped Map Examples

1. total_world_war.zip
2. total_world_war-master.zip

** If for some reason the XML mapName isn't in the format of using underscores, then the checks that use "-master" will convert it to have underscores while the other checks will use the mapName as is. So for example if the mapName was TotalWorldWar the following are valid:

1. TotalWorldWar
2. TotalWorldWar/map
3. total_world_war-master
4. total_world_war-master/map
5. TotalWorldWar.zip
6. total_world_war.zip
7. total_world_war-master.zip

[Cernel]

@redrum For the unzipped, I suggest keeping only

total_world_war
total_world_war-master/map

and making not working:

total_world_war/map
total_world_war-master

This way we keep only 2 standards, the basic / old and the GitHub one, instead of 4 different ways.

[redrum]

@cernel I don't think having support for those other 2 hurts anything. We can always just advertise in the map making guides the 2 which are most commonly used.

[Cernel]

@redrum Ok. It sounds a bit contradictory just mostly the "total_world_war/map" case, since the map is "total_world_war"; so having inside something called "map" makes sense if the other is the master of the map.

Also, I think that conversion thing is just confusing, and I suggest getting over it, by now. Meaning, not supporting conversion of "TotalWorldWar" to "total_world_war", but just going with it having to be exactly what is written in mapName (so, only "TotalWorldWar", for the master too, if that is the mapName). That looks just like 1 more thing to remember that is not necessary.

[Cernel]

@cernel Also, if possible, make the game never work if the mapName has any spaces, since you are supposed not having thoses, and it has been an issue, at times. Something like a crash with something telling "you must have no spaces in the mapName: use underscores, instead". Tho I'm not sure if this might be an issue for @RogerCooper ?

[redrum]

@cernel In general, the XML mapName should always just match the folder name and use underscores so "total_world_war". The only reason I think we left the conversion logic was in case some old maps that aren't in the official repo didn't follow that and had Proper case or used spaces.

[Cernel]

@redrum Correct and, as I said, I would get over that, and just force it being exactly the same; let people convert their maps properly. Would @RogerCooper have issues with this? I cannot think about any other problem really.
Also that really sounds like something that may bury inconsistent mapNames in the future, perpetuating them.

[Cernel]

@redrum As long as @RogerCooper assured and is assuring his maps to have exactly the same folder name and mapName, whatever the way, I think there is really no reason to keep that autoconversion.

[Cernel]

@redrum Also, in case you keep any autoconversion (I wouldn't), I'm wondering if turning upper case to lower case with a "_" before if any previous caracter is the only thing going on, or there are others (like turning spaces to a underscores and such).

[redrum]

@cernel Yeah, there is a bunch of stuff it tries around casing and spaces to try to get it to the github underscore format.

[Cernel]

@redrum I guessed so; so my suggestion is to get over all that, and just say mapName=folder name; period.
If @RogerCooper can confirm that is not killing him, I really think there are no reasons to keep those conversions going on.
What does @LaFayette think?

[redrum]

@cernel I don't really have a strong opinion and @LaFayette I believe is the one that originally created that. He may have more insight.

[Cernel]

@redrum It was really just to have a second ditch defence in the moment of the mass conversion, I think. Going on, you are just going to accept or not accept new maps to the repo singularly, and just need to make sure to tell that the map name needs:

1. no uppercase
2. no spaces (underscores yes)

But it depends if @LaFayette already did the full conversion of all the mapName entries in all the XML (tho, in the meantime, a few new maps might have slipped in that are using the autoconversion).