Return-To-The-Roots / s25client

Return To The Roots (Settlers II(R) Clone)
http://www.rttr.info
GNU General Public License v2.0
477 stars 75 forks source link

Add map selection to campaign lua #1662

Closed ottml closed 4 months ago

ottml commented 5 months ago

Add selection map screen to campaign lua and use it for world campaign. Add documention.

ottml commented 4 months ago

@Flamefire Could it be that you changed the hash? The ci is failing with this error:

Cloning into 'libsamplerate-src'...
fatal: unable to read tree (b5d39fbfc86b656fed68c935ac7b0a06349e4aa1)
CMake Error at libsamplerate-subbuild/libsamplerate-populate-prefix/tmp/libsamplerate-populate-gitclone.cmake:40 (message):
  Failed to checkout tag: 'b5d39fbfc86b656fed68c935ac7b0a06349e4aa1'
Flamefire commented 4 months ago

@Flamefire Could it be that you changed the hash? The ci is failing with this error:

I did some work in one repo and didn't realize it might cause some fallout. So yes this is fixed now.

During review I was a bit confused about the getRequiredLuaVersion vs campaign.version. It seems we require the exact version match of the former which means old lua files won't work. The latter seems to be unused.

IIRC we increase getRequiredLuaVersion only when the change is a breaking change. This one isn't so I think we can keep the current version. Or am I missing anything?

ottml commented 4 months ago

@Flamefire Could it be that you changed the hash? The ci is failing with this error:

I did some work in one repo and didn't realize it might cause some fallout. So yes this is fixed now.

During review I was a bit confused about the getRequiredLuaVersion vs campaign.version. It seems we require the exact version match of the former which means old lua files won't work. The latter seems to be unused.

IIRC we increase getRequiredLuaVersion only when the change is a breaking change. This one isn't so I think we can keep the current version. Or am I missing anything?

Yes the campaign.version is only for the campaign author an never used for checks.

I think we cannot stay at the current version. The former implementation for loading the campaingn description checked that all keys are read (luaData.checkUnused();) otherwise it failed. So the new format cannot be loaded by already existing builds. We have to increase the version for the world campaign. And change the version check to <= current version. So the new build can also load older versions.

Flamefire commented 4 months ago

Thanks a lot!