iAndyHD3 / WSLiveEditor

websocket server to interact with the geometry dash editor
5 stars 6 forks source link

Fix compilation errors, add 4 new action types #12

Closed kxtbit closed 3 months ago

kxtbit commented 4 months ago

For a while now, I have been using a custom modified version of this mod that I never published. Now that it was rewritten, I decided to add back my modifications and contribute them here for others to use.

This commit adds four new action types:

Other minor modifications are adding #include <Geode/Geode.hpp> at the top of each code file to resolve a lot of compilation errors I was running into where the compiler was trying to use stubbed headers instead of the real Geode bindings, and changing CMakeLists.txt to use src/actions/*.cpp to automatically include all of the files in src/actions instead of hardcoding them.

I'm not very experienced working with GitHub or contributing to others' code, so please let me know if I've done something wrong :)

iAndyHD3 commented 4 months ago

First of all, thank you for your work. I'm aware that more actions can be added, but I didn't add them on purpose. The reason is simple, I want the mod to be as easily portable as possible. Using more class members and gd functions means I will need to spend more time looking for the addresses and class members whenever a new update comes out. For example, a new GD update is set to release next week and it will require us to find all new functions and class members.

For that reason, I want to keep a low number of actions and only add ones that are absolutely necessary. The get next free group actions are exactly what I'm talking about, they need class members and they are something that can already be computed using the get level string action, for this reason I want to avoid adding these actions.

However, it might be possible to add some kind of arguments to the get level string action that computes next free groups from the level string itself and then returns it alongside the string (or not), this approach would not have any of the downsides I listed above (besides being obviously slower, as we need to get the entire level string instead of using class members)

That said, I do think there's a place for the get selected objects and the remove selected actions, however some implemention changes are needed.

About the actual code, what kind of errors were you getting and how are you building the mod? I want to avoid adding #include <Geode/Geode.hpp> because that includes every single geode header (that isn't needed) and I want to cut compile time as much as possible.

Moving forward, please remove get groups actions and rename REMOVE_SELECTED to REMOVE_SELECTED_OBJECTS. I will think of adding the next free groups in the level string action as I said above

kxtbit commented 4 months ago

I understand. I just made a commit to my branch to remove GET_NEXT_FREE_GROUPS and GET_NEXT_FREE_LINKS. Also, REMOVE_SELECTED was a mistake in my first message; the actual name of the action (in RemoveSelected.hpp) is already REMOVE_SELECTED_OBJECTS as you suggested.

As for the #include <Geode/Geode.hpp>, when I try to git clone your repo and build it with geode build (the only change I made was changing the Geode version to 2.0.0-beta.27 from 2.0.0-beta.26 in the mod.json to stop Geode from complaining that the version was wrong), MSVC errors out citing unknown size of SFXTriggerState. I will attach the full log here (with my personal details removed): https://pastecode.io/s/ude75k3u

If I do the same without changing the version in the mod.json, the log looks like this: https://pastecode.io/s/gvqx45tt

As far as I can tell, the issue is with GJGameState (and possibly other files too, but this is what shows up first in the error logs). LiveServer.cpp (and the other cpp files) includes LevelEditorLayer, which includes GJBaseGameLayer, which includes GJGameState. GJGameState defines a gd::vector of SFXTriggerState "m_sfxTriggerStates" but does not include SFXTriggerState.hpp. Because of this, the compiler instead resolves it to the empty stub definition in build/bindings/bindings/Geode/GeneratedPredeclare.hpp class SFXTriggerState; (at least, that's what my IDE shows me). It seems that, because the code for gd::vector internally needs to get the size of the value type at some point, the compiler then throws an error because the stub definition of SFXTriggerState has no size. This could be an issue with Geode itself, but I can't be entirely sure, and I don't want to go editing the Geode header files when I don't really know what I'm doing. I saw that a lot of the examples on the wiki had #include <Geode/Geode.hpp> at the top, so I tried adding that, and that fixed it since it imports the full definitions of all the classes. However, as you said, this increases compile time too. If for whatever reason this is just an issue on my side and you don't need the #include <Geode/Geode.hpp> to get it to build, maybe I should just remove it even if it doesn't work for me?

Again, sorry if I've made some mistake, and thanks!

iAndyHD3 commented 3 months ago

Thanks for making the changes. From now on if you don't mind I'll work further on your PR because I want to make some changes

The include error is strange, seems to have been fixed 3 months ago in this commit. Regardless I'll keep Geode.hpp if that fixes errors