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

Fix #6468: Load correct version of AI as specified during the time of its save. #7193

Open
wants to merge 1 commit into
base: master
from

Conversation

@SamuXarick
Copy link
Contributor

SamuXarick commented Feb 7, 2019

No description provided.

if (strcasecmp(ai_name, i->GetName()) == 0 && i->CanLoadFromVersion(versionParam) && (version == -1 || i->GetVersion() > version)) {
version = (*it).second->GetVersion();
info = i;
if (versionParam != -2) {

This comment has been minimized.

Copy link
@LordAro

LordAro Feb 23, 2019

Member

Not a fan of this "magic" -2 value. What's wrong with using versionParam "as is" ? It's documented as "The version of the game script, or -1 if you want the latest"

if (strcasecmp(ai_name, i->GetName()) == 0 && i->CanLoadFromVersion(versionParam) && (version == -1 || i->GetVersion() > version)) {
version = (*it).second->GetVersion();
info = i;
if (versionParam != -2) {

This comment has been minimized.

Copy link
@LordAro

LordAro Feb 23, 2019

Member

This is also basically identical to the GS version, I think differing only in AIInfo & GameInfo types? Can this be split out into a (templated?) function?

@SamuXarick SamuXarick force-pushed the SamuXarick:load-specific-ai-version branch from 2e3f65c to 5b40c05 Mar 1, 2019
@TrueBrain
Copy link
Member

TrueBrain commented Mar 2, 2019

Friendly poke; this is currently waiting on @SamuXarick (the author).

@SamuXarick
Copy link
Contributor Author

SamuXarick commented Mar 2, 2019

@SamuXarick
Copy link
Contributor Author

SamuXarick commented Mar 2, 2019

I need several versions of the same AI to re-test this. I have my own AI uploaded to bananas which would be useful, but I can only download the latest version. I thought I could download old versions at any time, so I deleted my old versions from disk.

@SamuXarick SamuXarick force-pushed the SamuXarick:load-specific-ai-version branch from 007f3aa to f932cd5 Mar 2, 2019
@SamuXarick
Copy link
Contributor Author

SamuXarick commented Mar 4, 2019

This is currently awaiting review, again

@stale
Copy link

stale bot commented Apr 3, 2019

This pull request has been automatically marked as stale because it has not had any activity in the last month.
Please feel free to give a status update now, ping for review, or re-open when it's ready.
It will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale label Apr 3, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:load-specific-ai-version branch from f932cd5 to e6ca553 Apr 7, 2019
@stale stale bot removed the stale label Apr 7, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:load-specific-ai-version branch 2 times, most recently from 87fba74 to 004843f Apr 11, 2019
@stale
Copy link

stale bot commented May 13, 2019

This pull request has been automatically marked as stale because it has not had any activity in the last month.
Please feel free to give a status update now, ping for review, or re-open when it's ready.
It will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale label May 13, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:load-specific-ai-version branch from 004843f to e7fac8c Jul 18, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:load-specific-ai-version branch from e7fac8c to 3edf435 Dec 30, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:load-specific-ai-version branch from 3edf435 to 425c863 Feb 9, 2020
@nielsmh
Copy link
Contributor

nielsmh commented Apr 7, 2020

I think we need a test case for this. The test case should take the form of two versions of an AI script where the second is an upgrade of the first, a save file which uses the first version, and a save file which uses the second version. The test would involve loading the first save and verifying the script version loaded, loading the second save and verifying the version loaded, then starting a new game with that AI enabled, and verifying it uses the latest version. Possibly also testing that the presence of other AIs does not affect this.

The test AI scripts for a test case should be written specifically for that test case, they should not be based off (or copies of) existing scripts.

@SamuXarick
Copy link
Contributor Author

SamuXarick commented Apr 8, 2020

PR7193 AI version tests savegame.zip
PR 7193 test AIs.zip

There are 4 versions of the same AI.
The savegame contains 5 AIs started. They were started like this:

  • startai pr7193
  • startai pr7193.1
  • startai pr7193.2
  • startai pr7193.3
  • startai pr7193.4

The first AI started as the latest available, which in this case it was version 4.
The other 4 AIs will try to load the exact version as the version they started with (1, 2, 3, 4), or the latest version that is compatible with the saved version.

Version 1 and 2 have a minimum load version 1. Version 3 and 4 have a minimum load version 3.

@SamuXarick SamuXarick force-pushed the SamuXarick:load-specific-ai-version branch from 425c863 to e2d1cd9 May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.