Vanilla-Conquer
Vanilla-Conquer copied to clipboard
Data path was not set to user path.
The data path was never set to the user path, when the data files were placed in the user path (e.g. on macOS and other Unix-based OS).
This is an improved solution for bug #795 and replaces PR #798.
I was able to implement a solution without the chdir call and prepare a branch now with that alternative solution for you to review.
Here is the alternative commit. Please review it and let me know, if I should create a pull request for it. Thanks! :-)
The new fix looks like a winner to me.
I have replaced the commits in this pull request with the new ones. Worked fine with force push as you suggested previously. ;-)
I rebased the branch again and squashed both commits into one to have it clean.
I had to push another commit.
The original method for loading single *.mpr skirmish / multiplayer maps in RA did only respect the working directory. So I added a loop that goes through all search paths and looks for these map files.
Edit: The second commit might not be Windows compatible due to the hardcoded "/" separator. I will fix it tomorrow and report back.
Good catch, there are still a few bugs with the new paths system, I've just noticed I fixed one in the PR I have open to move TD to a save system that is more similar to the RA system. Let me know when you want this properly reviewing as I'd like to run a few tests on it on the other platforms we support before merge.
I rebased again and improved the second commit. Now the OS dependent separator is used. Feel free to review and test. :-)
Edit: Found the next issue. Games are saved in the correct directory, but are not shown after saving in the load dialog. I will check that in the next days.
Edit 2: I think I already found a solution for the savegame issue. Should not be too difficult to implement. I suppose it will be ready until the end of the coming weekend.
I accidentally pushed my changes too early. The current branch is not ready yet. I will report back. :-)
Now, the work has been done. 😄
It contains the following Commits now:
- macOS.yaml had to get an emergency upgrade to macOS 12. 10.15 is failing from now on per default, because the availability will be removed end of August on GitHub.
- Data files are searched in all search paths. No more need for setting the working directory via chdir.
- Also the single skirmish / multiplayer maps (*.mpr) for Red Alert are searched in all search paths.
- Savegames can be loaded, saved and deleted. They are expected to be placed in the same directory as the ini file (e.g. "RedAlert.ini").
While testing the fourth point, I found another bug, which is unrelated to my changes. It also occurs without all these changes. The loading dialog of Red Alert is only showing the first savegame, but internally all savegame files are parsed correctly and put into the internal file list. Seems to be a visualization issue. I will create a bug report for that and do the investigations in a few days. (#819)
Please feel free to test and review it. Hopefully no further bugs will be found that are related to my commits.
This is looking good, I'm just a bit busy with real life at the moment, but once I get chance to build and test this locally I think its good to merge.
Bad news, this breaks the windows build because fopen doesn't return a valid FILE pointer when called on a directory so CCFileClass never successfully finds the directories that contain the CD data. Instead I think the find logic to scan all paths in the search path needs implementing in Change_Local_Dir and the file class to test needs passing the absolute path to test for being a directory.
Is it only the commit for finding the mix files? Maybe I have an idea how to adapt it by finding one main mix file by the same method like the mpr maps or savegames and then adjusting the absolute path of it by adding the subfolders to it for checking if they exist. I may find some time to work on it in the next days. Unfortunately, I have currently no Windows machine to test.
Edit: I checked the method you have mentioned and now I am sure our ideas head into the same direction.
I have now implemented a data directory search method (see latest commit here). I was also able to get a Windows running laptop for quick tests but unfortunately it is also not sufficient to hand over the full path in the change_local_dir method. It is again working on macOS, but not on Windows.
Shouldn't we go for the most simple solution first, to make it running on all operation systems by adjusting the working directory on startup via chdir as I did it in the beginning? We would be then able to provide working versions for Windows and Unix systems and concentrate on the bigger solution to get it working without chdir afterwards.
I updated my personal forked main branch with this commit as I had it previously some days before. In my opinion, we should go for this commit first, before working further on an improved data file and savegame handling without the chdir command in the code. With this commit, we would have a version on the master branch here that runs on Windows and unix operation systems without any major problems.
With the new VMware Fusion Tech Preview for M1 Macs that has been released recently, I was able to test the mentioned commit and the resulting binaries on macOS, Linux and Windows successfully. For macOS and Linux I compiled native ARM64 versions and on Windows 11 ARM I have ran the x64 binaries.
If you agree, I will update the branch for this pull request with only the one commit (all others are not necessary then). Afterwards I can then check the improved approach again and how to also make it runnable in Windows.
I appreciate that your commit does fix the issue, but it doesn't work as the logic was intended when I wrote it. I've wrote a gist that should search for the folders in all the possible data locations though which will hopefully fix it more in a way I wanted it to work in the first place: https://gist.github.com/OmniBlade/f1e4257d3d25153fa0e00a72fa7c331b
That looks similar to the new approach I tried before. Unfortunately my new approach will not work either on Windows. But I did not test your variant yet.
I've tested the fix in RA and it worked for windows and I don't see why it wouldn't for other platforms.
I might be able to also test it the next days.
I have updated the branch of this PR with your latest code changes committed. This branch contains now a necessary update for the macOS action, the code changes for finding the data files and skirmish maps without chdir and two of your bugfixes for the save game loading. For working savegame loading and saving in TD, your third commit from PR #715 would be also necessary.
I tested your latest code suggestion on macOS 12.5 and Windows 11 ARM.