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.

Sign up for GitHub

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

Jump to bottom

game converter for old linux games to the new unified format for both linux and windows #77

Merged
merged 11 commits into from
Feb 1, 2019
Merged

game converter for old linux games to the new unified format for both linux and windows #77

merged 11 commits into from
Feb 1, 2019

Conversation

LynxAbraxas
Copy link
Contributor

PR relating to #70, with the following commits:

  1. revert to load and save the old style format for linux

  2. linux save-game in the old format that loads, and if saved should be loadable again.

  3. Changes to save in the new format but still load the old, i.e. the converter version of ctp2 for old linux save-games to the new format that is then compatible with windows save-games.

  4. the save-game from 2. should still be loadable, but if saved the new version should not be loadable any more (but instead with ctp2 from master), so just as a reference, the newly saved game is added as a commit.

Note: This PR is then intended to not be merged into master but instead into a new branch that then gets a tag to be the converter release.

@MartinGuehmann
Make c3.h to includes what it is supposed to include
c5dd5ed 
On Linux c3.h is supposed to include ctp2_config.h, which includes config.h, but only if HAVE_CONFIG_H has been defined. This is however defined in windows.h, which was included afterwards. ctp2_config.h was already included by ctp2_inttypes.h, since both files have multiple inclusion guards, things that should be defined in the translation unit won't be defined.

For instance if you build a debug build, _DEBUG should be defined, but because of this you can't be sure whether it is really defined. This is also true for USE_SDL.

Obviously, this small change can change what is compiled at various places. Actually, c3.h is supposed to be a precompiled header that is supposed to be included in every source. However, this is not true, even so it must be done to make sure that config.h is used in every translation unit.

..\ctp2_code\ctp\c3.h
..\ctp2_code\os\include\ctp2_inttypes.h
@MartinGuehmann
Fix the include order so that it also compiles on Linux
8cc0263 
..\ctp2_code\ctp\c3.h
..\ctp2_code\os\nowin32\windows.h
@MartinGuehmann
Merge branch 'master' of https://github.com/civctp2/civctp2 into Fix-…
7b680be 
…Include-Order-In-c3.h
@MartinGuehmann
Merge branch 'master' of https://github.com/civctp2/civctp2 into Fix-…
8dd104d 
…Include-Order-In-c3.h
@MartinGuehmann MartinGuehmann added the WIP A pull request that is work in progress, the maintainer shouldn't merge it, yet label Jan 28, 2019
@LynxAbraxas
Copy link
Contributor Author

@MartinGuehmann Commits OK for you? Can you do 3.? For that just add a remote to my fork:

git remote add lynx https://github.com/LynxAbraxas/civctp2
git fetch lynx
git checkout lynx/gameConverter -b gameConverter

and add your changes onto that and create a PR to my fork and I can include it in this PR or you can create a new PR or use your maintainer power and directly push the new branch to https://github.com/civctp2/civctp2.

Or you can do this: https://help.github.com/articles/checking-out-pull-requests-locally/#modifying-an-inactive-pull-request-locally

@LynxAbraxas LynxAbraxas changed the title game converter for old linux games to the new unified format for both linux and windows WIP: game converter for old linux games to the new unified format for both linux and windows Jan 28, 2019
@MartinGuehmann MartinGuehmann changed the base branch from master to SaveGameConverter January 28, 2019 23:45
@MartinGuehmann
Copy link
Collaborator

You could have left out citydata.h, UnitData.h, and Vision.h, but if they do not confuse you anymore then they can be left in.

I figured out how to create a new branch in the repository via the GitHub interface. So I changed the merge target of this pull request.

I can do 3.

The files to modify are:

c3.h
windows.h
GameFile.cpp
CityData.cpp
Player.cpp

Player.cpp is the harder one, so probably, not today. Probably, I can still use #70 as a test PR.

@LynxAbraxas
Copy link
Contributor Author

You could have left out citydata.h, UnitData.h, and Vision.h

As I understood your explanation, they do concern that commit, so I left them in.

I figured out how to create a new branch in the repository via the GitHub interface. So I changed the merge target of this pull request.

That's great, looks good.

Probably, I can still use #70 as a test PR.

Sure, you can just push to #70 and I can then cherry-pick your commit on to this PR.

@LynxAbraxas
Copy link
Contributor Author

Hm, wouldn't it suffice to revert changes from afb53bf since it fully reverts to the old style?
I.e. afb53bf#diff-8112044a382ecd37162d665f1e425877L1401
and to adjust
afb53bf#diff-b8ab958d11c40fd714d3d01a8c6a2614
and the Serialize from
afb53bf#diff-647de8d8715871ce5f3623bf0a4f0158
to distinguish between loading and saving?

Player.cpp wasn't touched at all in afb53bf.

@LynxAbraxas
Copy link
Contributor Author

wouldn't it suffice to revert changes from afb53bf since it fully reverts to the old style?

Like in ef5d674?
Not sure though if the GUID needs to be handled differently on load and save.

@LynxAbraxas
Copy link
Contributor Author

OK, with ef5d674 I can still load the old games and save the to a new format but that can neither be opened with the same exe nor with that from master, so I guess ef5d674 is missing some changes, possibly differentiating the GUID on save and load. @MartinGuehmann any ideas?

@MartinGuehmann
Copy link
Collaborator

It's the GUID that makes the trouble.

@LynxAbraxas
Copy link
Contributor Author

@MartinGuehmann Do you have an idea how to solve that for the load-old save-new format commit?

@LynxAbraxas LynxAbraxas force-pushed the gameConverter branch 3 times, most recently from 14152d5 to 3382865 Compare January 31, 2019 18:43
@LynxAbraxas
Copy link
Contributor Author

Squashed your new commits to #70 and separated the white space changes into their own commit, that way it is far easier to rule out commits for future debugging.

Works perfectly, can now convert old Linux save-games to the new unified format, see e.g. fd25b12. Many thanks @MartinGuehmann for this special effort!

@MartinGuehmann What do you think? From my side ready for the game-converter tag/release.

@LynxAbraxas LynxAbraxas changed the title WIP: game converter for old linux games to the new unified format for both linux and windows game converter for old linux games to the new unified format for both linux and windows Jan 31, 2019
@LynxAbraxas LynxAbraxas mentioned this pull request Jan 31, 2019
Closed
@MartinGuehmann
Copy link
Collaborator

MartinGuehmann commented Jan 31, 2019 •
edited
Loading

Squashed your new commits to #70 and separated the white space changes into their own commit, that way it is far easier to rule out commits for future debugging.

Well, the white space, it is simply done by the way. So it goes in by the way.

@MartinGuehmann What do you think? From my side ready for the game-converter tag/release.

From my side it is done two and could go to its dedicated branch. Unless you still wanna update the readme. Which we could also do in principle in a later pull request.

@MartinGuehmann MartinGuehmann removed the WIP A pull request that is work in progress, the maintainer shouldn't merge it, yet label Jan 31, 2019
@MartinGuehmann
Merge pull request civctp2#78 from MartinGuehmann/Fix-Include-Order-I…
582131e 
…n-c3.h

Make c3.h to includes what it is supposed to include
@LynxAbraxas
Copy link
Contributor Author

Unless you still wanna update the readme. Which we could also do in principle in a later pull request.

Good point, better now than later (or never).
URL for the game converter release expects the git tag to be GameConverter, intentionally not the same as the branch, because it can be troublesome to have the same ref-name for a branch and tag.
@MartinGuehmann After merging the PR, give it the tag name GameConverter with an additional message. That will turn it into a GH release.

MartinGuehmann and others added 4 commits February 1, 2019 13:41
@MartinGuehmann @LynxAbraxas
Load and save games in the old format under linux (pre-stage for game…
4fd8177 
…Converter release):

This commit basically reverts back to the state before 22dd180 concerning the save-game format for linux
created from the commits of civctp2#70 with:
git merge --squash b475d51
git diff -U0 -w --no-color | git apply --cached --ignore-whitespace --unidiff-zero -

Squashed commit of the following:

commit b475d51
Author: Martin Gühmann <guehmann@t-online.de>
Date:   Sun Jan 27 21:10:40 2019 +0100

    Fix the last incompatibility for loading Linux games in the old format
    ..\ctp2_code\gs\gameobj\CityData.cpp

    Adds some test code that is not so trivial to generate. It is however outcommented.
    ..\ctp2_code\gs\gameobj\UnitData.h
    ..\ctp2_code\gs\gameobj\Vision.h
    ..\ctp2_code\gs\gameobj\citydata.h

    Just a style change, no functional change, is for now also outcommented
    ..\ctp2_code\os\nowin32\windows.h

commit dcd2f41
Merge: dbd2603 57f0b0d
Author: Martin Gühmann <guehmann@t-online.de>
Date:   Sun Jan 27 00:20:27 2019 +0100

    Make it compile
    ctp2_code/os/nowin32/windows.h

commit dbd2603
Author: Martin Gühmann <guehmann@t-online.de>
Date:   Sun Jan 27 00:04:54 2019 +0100

    Make games load that have been created before the format fix, the games are saved in the same format
    ..\ctp2_code\ctp\c3.h
    ..\ctp2_code\gs\fileio\GameFile.cpp
    ..\ctp2_code\os\nowin32\windows.h

commit 57f0b0d
Author: Martin Gühmann <guehmann@t-online.de>
Date:   Sun Jan 27 00:04:54 2019 +0100

    Make games load that have been created before the format fix, the games are saved in the same format
    ..\ctp2_code\ctp\c3.h
    ..\ctp2_code\gs\fileio\GameFile.cpp
    ..\ctp2_code\os\nowin32\windows.h
@LynxAbraxas
save game in the old linux format for testing the gameConverter
a4b2035
@MartinGuehmann @LynxAbraxas
Load games in the old format under Linux and save to the new format c…
fe41371 
…ompatible with Win (for gameConverter release):

Squashed commit of the following (from civctp2#70, without white space changes):

commit 1b096f4e8eccb6bdc295fd1943507f7daaddc957
Author: Martin Gühmann <guehmann@t-online.de>
Date:   Wed Jan 30 23:05:21 2019 +0100

    Make games saved indeed in the Windows format

    Also k_SCENARIO_NAME_MAX is defined as _MAX_PATH, which makes it hard to catch.
    ..\ctp2_code\gs\fileio\GameFile.cpp

commit 19ea18f397807ff7b481d0847af1a54846788e43
Author: Martin Gühmann <guehmann@t-online.de>
Date:   Wed Jan 30 20:26:54 2019 +0100

    Make CTP2 load games in the old Linux format and save them in the Windows format
    ..\ctp2_code\gs\fileio\GameFile.cpp
    ..\ctp2_code\gs\gameobj\CityData.cpp
    ..\ctp2_code\gs\gameobj\Player.cpp
    ..\ctp2_code\os\nowin32\windows.h
@LynxAbraxas
white space changes only from b475d51 and 73561c5
96f9a47
@LynxAbraxas
Copy link
Contributor Author

Rebased on to current master.

@MartinGuehmann MartinGuehmann merged commit 77ad5c2 into civctp2:SaveGameConverter Feb 1, 2019
@MartinGuehmann
Copy link
Collaborator

So merged and released: https://github.com/civctp2/civctp2/releases/tag/GameConverter, via the GitHub web interface. I can still edit the release, if you don't like the text, which I mainly took from the readme, or wanna have some binaries added.

@LynxAbraxas
Copy link
Contributor Author

Looks good, thanks @MartinGuehmann

@LynxAbraxas
Copy link
Contributor Author

LynxAbraxas commented Feb 1, 2019 •
edited
Loading

@MartinGuehmann Sorry, commented to quickly, just noticed that the tag does not point to the right commit. It should point to 77ad5c2

@LynxAbraxas LynxAbraxas deleted the gameConverter branch February 1, 2019 20:05
@MartinGuehmann
Copy link
Collaborator

That's odd, I expect if I select the correct branch that it contains the last thing I added. So deleted the release and tag and readded it. The odd thing is when I tried to select a commit, it did not appear there. But then it worked. Maybe these annoying messages "...added an hour from now." have some meaning. You know when I read it it means too me that the time is in the future, even so it is in the past.

Maybe I have to wait an hour until I can select it.

@LynxAbraxas
Copy link
Contributor Author

Perfect now.

This was referenced Feb 1, 2019
Closed
Closed
@MartinGuehmann MartinGuehmann mentioned this pull request Feb 4, 2019
Merged
@LynxAbraxas LynxAbraxas mentioned this pull request Feb 8, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@LynxAbraxas @MartinGuehmann