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

Update compatibility to 4.0-stable #304

Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open

Conversation

Beliar83
Copy link
Contributor

@Beliar83 Beliar83 commented Oct 31, 2022

This makes godex compile with commit 92bee43adba8d2401ef40e2480e53087bcb1eaf1, which is the one that was used to build the stable reelase of Godot 4.0.

I am not 100% if the function I wrote to replace remove_and_skip is working correctly. I did some quick tests and it looks okay, but maybe someone with a deeper knowledge of godex should double check.

@Beliar83 Beliar83 changed the title Update compatibility for 4.0 beta3 Update compatibility for 4.0 rc1 Feb 11, 2023
@Beliar83
Copy link
Contributor Author

I have added changes for compatibility for up to 4.0 rc1.
I also added a fix for register_runtime_scripts that does not need a godot change.

Handle WorldECSEditorPlugin::edit being called with nullptr
Move adding Components3DGizmoPlugin to _editor_init
Reset default properties when destructing ScriptEcs
Remove explicit -j option and let the godot build system automatically find the best
@Beliar83
Copy link
Contributor Author

@RevoluPowered Where exactly are the extra whitespaces? Or is there one missing?

@mkay-dev
Copy link

mkay-dev commented Apr 11, 2023

This makes godex compile with commit 92bee43adba8d2401ef40e2480e53087bcb1eaf1, which is the one that was used to build the stable reelase of Godot 4.0.

Please continue updating Godex while Andrea Catania is busy. There seem to be new issues with compilation due to the following Godot commit godotengine/godot@4e34cf2 .

@Beliar83
Copy link
Contributor Author

@mkay-dev Ah, I guess that was the error on the build I got earlier. I'll try to check that when I am home later today.

@mkay-dev
Copy link

mkay-dev commented Apr 11, 2023

@Beliar83 Thanks in advance. Can I maybe also motivate you to fix some linux issues should they arise. I'm not yet deeply familiar with c++ and godot/godexs source code so I'm only working up to fixing these kind of issues myself.

@Beliar83
Copy link
Contributor Author

Beliar83 commented Apr 11, 2023

@mkay-dev It should generally build on linux (the automatic builds in my fork and this PR do), though I have WSL or a VM at best. If you have any specific problem I can look at it, though.

@mkay-dev
Copy link

@Beliar83 Sounds great. Your fork did compile against Godots 4.0 release build just fine so I'm just being heedful of possible issues. Again thanks for the help.

@mkay-dev
Copy link

mkay-dev commented Apr 12, 2023

Here is the compilation error I am getting:

[ 7%] Generating modules tests header.
[ 26%] Compiling modules/gdscript/gdscript_byte_codegen.cpp ...
[ 26%] Compiling tests/test_main.cpp ...
[ 26%] Compiling modules/gdscript/gdscript_cache.cpp ...
[ 26%] Compiling modules/gdscript/gdscript_compiler.cpp ...
In file included from /home/mkay/projects/godot/from_scratch/modules/godex/tests/test_ecs_pipeline_builder.h:14,
from ./modules/modules_tests.gen.h:12,
from tests/test_main.cpp:113:
/home/mkay/projects/godot/from_scratch/modules/godex/tests/test_utilities.h: In function 'bool register_ecs_script(const StringName&, const String&)':
/home/mkay/projects/godot/from_scratch/modules/godex/tests/test_utilities.h:32: warning: comparison with string literal results in unspecified behavior [-Waddress]
32 | if (extends == "System") {
|
/home/mkay/projects/godot/from_scratch/modules/godex/tests/test_utilities.h:32: error: comparison between distinct pointer types 'GDScriptParser::IdentifierNode*' and 'const char*' lacks a cast [-fpermissive]
[ 26%] Compiling modules/gdscript/gdscript_disassembler.cpp ...
scons: *** [tests/test_main.linuxbsd.editor.x86_64.o] Error 1
scons: building terminated because of errors.
[Time elapsed: 00:00:38.732]

@Beliar83
Copy link
Contributor Author

Beliar83 commented Apr 12, 2023

@mkay-dev I reset my fork and worked on a separate branch for updating the compatibility - as I should have done from the start, so there was nothing changed yet for this PR. I have fixed the problem, but I am doing one more check build after merging from this repo.

@mkay-dev
Copy link

@Beliar83 Excellent! You're a true hero.
Seems to be compiling fine for now. Only had to add missing #include "utility" in /modules/godex/tests/test_ecs_storage_hierarchical.h and /modules/godot/nodes/ecs_world.cpp as I was getting:

error: 'as_const' is not a member of 'std'; did you mean 'is_const'?

@fire
Copy link
Contributor

fire commented Jun 6, 2023

Typically godot engine developers do one merge commit (rebase and then force push to a feature branch). What does @AndreaCatania prefer and how far is this from working?

@fire
Copy link
Contributor

fire commented Jul 9, 2023

Godot 4.1-stable released a few days ago.

@fire fire mentioned this pull request Jul 9, 2023
@fire
Copy link
Contributor

fire commented Jul 9, 2023

https://github.com/Beliar83/godex/blob/main/systems/system.h#L6 is using the wrong type of include.

#include "../ecs_types.h"

@Beliar83
Copy link
Contributor Author

Yeah, I have already updated it in https://github.com/Beliar83/godex/tree/dev_main

@YPares
Copy link

YPares commented Oct 11, 2024

Hi! Is the work on that PR still going on?

@AndreaCatania
Copy link
Member

Hi @YPares, I don't think so.

@YPares
Copy link

YPares commented Oct 11, 2024

Oh :/ ok. That's too bad...

@Beliar83
Copy link
Contributor Author

@YPares If you mean whether I still do updates, then no.

@fire
Copy link
Contributor

fire commented Oct 11, 2024

If there is still interest we'll probably need to fork the repository and start merging changes.

@AndreaCatania
Copy link
Member

This is a good idea @fire

@fire
Copy link
Contributor

fire commented Oct 11, 2024

@AndreaCatania since you're here, you could give maintainer ship to interested people wanting to maintain godex

@AndreaCatania
Copy link
Member

Hey @fire, fork the project is the way to go, at least for the moment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants