Merge an updated godot4 migration to the base branch#205
Merged
qrrk merged 33 commits intoqrrk:godot4from Oct 4, 2025
Merged
Conversation
Co-authored-by: unknown <jedibob5@yahoo.com>
fix qrrk#180 change cdda-*-tiles-x64 to cdda-*-with-graphics-and-sounds-x64
…hy it was expecting an EN in CS folder
Not sure why it was being set during runtime, but set_icon is no longer available, and .svg files are not supported only .png.
Owner
|
Hey @henry-thomson ! Thanks for the updates and sorry about disappearing. I think I might be able to continue working on this soon. I've looked at your changes, and everything seems reasonable. I think I'll just merge it into my branch and go from there. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I noticed that you started off migration to godot 4, and I would like to finish it before adding any features. This PR extends the original from #167 so that it is easier to review.
I have a question though, I am able to run multiple versions of CDDA, so I assume the
points from #167 (comment) is finished.
However, I have literally just rebased his PR onto master as it was missing some commits, and then started removing any runtime errors that stopped me from launching the game, and even though these are minimal changes the total changes are 55 files. Majority are deletions, and stuff already merged into master from the rebase, but still not simple to review.
I am happy to work on the rest of the
MUST HAVEfeatures in themes and scaling, but I worry I will make it harder to review.I assume the question is mostly at @AriaMoradi as I have no idea how active @qrrk is. Do you prefer this single PR that fulfills all
MUST HAVE, or should I wait for this PR to be merged into the godot4 one, and then I continue with the rest of the features? What are your thoughts?Do we have someone to test Windows release? I have only Linux available ATM. I assume you can test my PR if need be as I don't see any CI files, and so assume you are building everything manually?
A warning if you run catapult with no configuration, it will most likely fail or seem like it is not working. However, if you just close it, and reopen a few times, the default values somehow do get populated, and you won't be getting the issues any more. I think this should get fixed before the godot 4 PR is merged, but for now the somewhat happy path works, and you can start CDDA.
Also, do you know anything about @qrrk ? He seems to have very sparse activity. I think in one of the docs I read he is likely to lose interest half way through the projects, and I worry that the godot migration broke him 💀 Are there any more reviewers?
Hopefully the commits are easy to read by themselves, but here is extra context if needed, and also for me if I forget why I changed something:
Changes
Removal of SVG files,
.translation, and imports. I am not sure what was causing these files to have incorrect imports, but when I opened the project in godot I had ~50 import errors. Regardless of what I did to fix the error message, the only improvement was deletion of the.importfiles, which forced godot to regenerate the import files, but then everything had default import settings anyway, and it added lots of code to review. My best guess is that the imports were used previously, but at some point textures were generated from them, and thus the SVG files never got used. Maybe during the migration this data got lost, and godot was unable to properly handle them, but the texture files did get migrated and work properly. As for the translation files, I don't think they were used anywhere, and one was incorrectly named? I would love to readd the files later on, but for now they are at least kept in the git history.OS.get_name()now returnsLinuxinstead ofX11, not sure about every possible use case, and I believe godot 4 started offering aDisplayServerfor handling that. So to avoid any larger changes I have simply duplicated the functionality, and it will be removed later.Default icon fix, the
set_iconfunction doesn't exist in godot 4, and the intended way to set the icon is through project settings instead. However, only PNGs are accepted, so I exported the SVG as PNG with Gimp.center_windowdoesn't exist in godot 4 and there is no replacement AFAIK, so I removed it. For now I don't think it is an issue that the first time you run catapult you won't have the window centered, as I will have to look at the implementation of the center_window and then reimplement it myself. Should be simple, but again more code for review than is necessary.I am not sure why @qrrk didn't consider
Handling of multiple game installationspoint finished, as I was able to run multiple experimental CDDA versions just fine. However, I did notice that you are not able to run older versions if you run the newer versions. After some debugging, I found the following error from the CDDA itself.I believe it is caused by CleverRaven/Cataclysm-DDA#78830 as they changed the font config expectations(?) and they allowed the migration from old to new, but not from new to old. This means if you run experimental, and then want to use stable, catapult will just hang for a second and then silently crash. Unfortunately, we are not checking the exit code right now, nor the
stderroutput. I don't think this is an issue with the migration though as this doesn't work even in godot 3 version. I am pretty sure we can fix it somehow eventually.