godotjs / javascript

Javascript binding for godotengine
https://godotjs.github.io/
MIT License
980 stars 82 forks source link

Refactor repo branches to enable good publish process #165

Closed nmerget closed 1 month ago

nmerget commented 1 year ago

We should do some changes:

fire commented 1 year ago

Master should use 4.1 now.

fire commented 1 year ago

I am testing builds on 4.1 Godot Engine https://github.com/V-Sekai/godot/tree/vsk-javascript-4.1

fire commented 1 year ago

It fails to build on Linux etc.



modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:30:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.u.ptr = p_function.javascript_object;
        ~~~~~~~^~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:31:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.tag = JS_TAG_OBJECT;
        ~~~~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:41:7: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
                temp.u.ptr = js_function.javascript_object;
                ~~~~^~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:42:7: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
                temp.tag = JS_TAG_OBJECT;
                ~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:50:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.u.ptr = js_function.javascript_object;
        ~~~~~~~^~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:51:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.tag = JS_TAG_OBJECT;
        ~~~~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:58:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.u.ptr = js_function.javascript_object;
        ~~~~~~~^~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:59:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.tag = JS_TAG_OBJECT;
        ~~~~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:72:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.tag = JS_TAG_OBJECT;
        ~~~~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:73:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.u.ptr = js_function.javascript_object;
        ~~~~~~~^~
10 errors generated.
scons: *** [modules/javascript/thirdparty/quickjs/quickjs_callable.android.template_release.arm32.o] Error 1
modules/javascript/thirdparty/quickjs/quickjs_debugger.cpp:82:24: error: out-of-line definition of 'connect' does not match any declaration in 'QuickJSDebugger'
Error QuickJSDebugger::connect(JSContext *ctx, const String &address) {
                       ^~~~~~~```
nmerget commented 1 year ago

Okay, I have several questions:

  1. Why did you merge a branch with a failed pipeline into master? The master branch should be protected by status checks which can't be bypassed to make sure that there is at least one valid branch. The PR should be reverted until the 4.1 is fixed.

  2. Before merging 4.1 to master, we should open a new branch 3.4. I don't know if someone is using this version for a "real" project, but we should make sure that we are still aligned with the major versions of godot.

  3. For https://github.com/V-Sekai/godot, we should make sure that we enable other builds for linux, which require some changes, but this should be another issue.

  4. You changed/reverted a lot of the branch inside my fork. It's pretty hard to resolve what's going on now. Maybe we can rollback everything to make sure everything is tested?

fire commented 1 year ago
  1. The failed pipeline isn't enabled until the ci can be merged. so we cannot see the effects until it is merged
  2. I'll do that immediately. ✅
  3. https://github.com/V-Sekai/godot - I had no way of testing this here because of (1), so I did it on the v-sekai orgination branch
  4. I'll do this in about an hour

a. copy the last 3.4 build to the branch 3.4 ✅ b. the last commit should be 1 commit on top of gd4 next c. I'll rollback in one hour to this layout:

  1. previous master -> 3.4
  2. current master -> 4.1 (I will pretend 4.2 doesn't exist) ✅
  3. you can look at the gd4 for the commits. let me know if that is ok
fire commented 1 year ago

To be clear. I'll rollback master to the last 3.4 in an hour.

fire commented 1 year ago

@nmerget Visual Studio compiler diverged from the godot 3 version and the cicd breaks.

nmerget commented 1 year ago

I reverted my initial branch to compare the differences between it and the current master. And I have some questions:

Why did you change the .github folder again? Basically dropped all my changes and keep the copy/paste from godotengine. Was this your purpose? Furthermore, the pipeline fails in master because of the wrong indent in this file.

In addition, I'm confused why you kept quickjs/quickjs_callable.cpp in this directory. It would make sense to me to:

  1. move it to thirdparty/quickjs to have everything in one place
  2. create a new directory bindings to with another directory quickjs so we separate thirdparty and bindings

--> I think the main task is to fix the pipeline(s) again and align all the branches, and we shouldn't copy/paste things from godotengine unless it makes sense

fire commented 1 year ago

@nmerget I have been giving less effort to this project. @Geequlim I would suggest giving commit access to @nmerget as he's been doing a good job untangling this. Too many things going on.

You have my blessing to maintain the project.