Skip to content

Commit e3b9ad1

Browse files
bors[bot]Bromeon
andauthored
Merge #215
215: Contribution guidelines, update `.gdextension` configurations r=Bromeon a=Bromeon Elaborates the contribution process, documents `#[itest]` and unifies the `.gdextension` library entries. Fixes #208. Co-authored-by: Jan Haller <[email protected]>
2 parents 77ca78d + 11f070b commit e3b9ad1

File tree

5 files changed

+161
-53
lines changed

5 files changed

+161
-53
lines changed

.github/workflows/full-ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

55
# Full CI workflow
6-
# Run before merging. Rebases on master to make sure CI passes for latest integration, not only for the PR at the time of creation.
6+
# Runs before merging. Rebases on master to make sure CI passes for latest integration, not only for the PR at the time of creation.
77

88
name: Full CI
99

Contributing.md

Lines changed: 129 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,175 @@
1-
# Contributing to `gdext`
1+
# Contributing to gdext
22

3-
At this stage, we appreciate if users experiment with the library, use it in small projects and report issues and bugs they encounter.
3+
We appreciate if users experiment with the library, use it in small projects and report issues they encounter.
4+
If you intend to contribute code, please read the section _Pull request guidelines_ below, so you spend less time on administrative tasks.
45

5-
If you plan to make bigger contributions, make sure to discuss them in a [GitHub issue] first. Since the library is evolving quickly, this avoids that multiple people work on the same thing or implement features in a way that doesn't work with other parts. Also don't hesitate to talk to the developers in the `#contrib-gdext` channel on [Discord]!
6+
The rest of the document goes into tools and infrastructure available for development.
67

7-
## Check script
88

9-
The script `check.sh` in the project root can be used to mimic a CI run locally. It's useful to run this before you commit, push or create a pull request:
9+
## Pull request guidelines
1010

11-
```
12-
$ check.sh
11+
### Larger changes need design
12+
13+
If you plan to make bigger contributions, make sure to discuss them in a [GitHub issue] before opening a pull request (PR).
14+
Since the library is evolving quickly, this avoids that multiple people work on the same thing, or that features don't integrate well,
15+
causing a lot of rework. Also don't hesitate to talk to the developers in the `#contrib-gdext` channel on [Discord]!
16+
17+
18+
### One commit per logical change
19+
20+
This makes it easier to review changes, later reconstruct what happened when and -- in case of regressions -- revert individual commits.
21+
The exception are tiny changes of a few lines that don't bear semantic significance (typos, style, etc.).
22+
Larger code style changes should be split though.
23+
24+
If your pull request changes a single thing, please squash the commits into one. Avoid commits like "integrate review feedback" or "fix rustfmt".
25+
Instead, use `git commit --amend` or `git rebase -i` and force-push follow-up commits to your branch (`git push --force-with-lease`).
26+
Since we use the _bors_ bot to merge PRs, we can unfortunately not squash commits upon merge.
27+
28+
29+
### Draft PRs
30+
31+
In case you plan to work for a longer time on a feature/bugfix, consider opening a PR as a draft.
32+
This signals that reviews are appreciated, but that the code is not yet ready for merge.
33+
Non-draft PRs that pass CI are assumed to be mergeable (and maintainers may do so).
34+
<br/>
35+
36+
# Dev tools
37+
38+
## Local development
39+
40+
The script `check.sh` in the project root can be used to mimic a minimal version of CI locally.
41+
It's useful to run this before you commit, push or create a pull request:
42+
43+
```bash
44+
$ ./check.sh
1345
```
1446

15-
At the time of writing, this will run formatting, clippy, unit tests and integration tests. More checks may be added in the future. Run `./check.sh --help` to see all available options.
47+
At the time of writing, this will run formatting, clippy, unit tests and integration tests. More checks may be added in the future.
48+
Run `./check.sh --help` to see all available options.
1649

1750
If you like, you can set this as a pre-commit hook in your local clone of the repository:
1851

19-
```
52+
```bash
2053
$ ln -sf check.sh .git/hooks/pre-commit
2154
```
2255

23-
## Unit tests
2456

25-
Because most of `gdext` interacts with the Godot engine, which is not available from the test executable, unit tests (using `cargo test` and the `#[test]` attribute) are pretty limited in scope.
57+
### Unit tests
2658

27-
Because additional flags might be needed, the preferred way to run unit tests is through the `check.sh` script:
59+
Because most of gdext interacts with the Godot engine, which is not available from the test executable, unit tests
60+
(using `cargo test` and the `#[test]` attribute) are pretty limited in scope. They are primarily used for Rust-only logic.
2861

29-
```
62+
As additional flags might be needed, the preferred way to run unit tests is through the `check.sh` script:
63+
64+
```bash
3065
$ ./check.sh test
3166
```
3267

33-
## Integration tests
34-
35-
The `itest/` directory contains a suite of integration tests that actually exercise `gdext` from within Godot.
3668

37-
The `itest/rust` directory is a Rust `cdylib` library project that can be loaded as a GDExtension in Godot, with an entry point for running integration tests. The `itest/godot` directory contains the Godot project that loads this library and invokes the test suite.
69+
### Integration tests
70+
71+
The `itest` directory contains a suite of integration tests. It is split into two directories:
72+
`rust`, containing the Rust code for the GDExtension library, and `godot` with the Godot project and GDScript tests.
73+
74+
Similar to `#[test]`, the function annotated by `#[itest]` contains one integration test. There are multiple syntax variations:
75+
76+
```rust
77+
// Use a Godot API and verify the results using assertions.
78+
#[itest]
79+
fn variant_nil() {
80+
let variant = Variant::nil();
81+
assert!(variant.is_nil());
82+
}
83+
84+
// TestContext parameter gives access to a node in the scene tree.
85+
#[itest]
86+
fn do_sth_with_the_tree(ctx: &TestContext) {
87+
let tree: Gd<Node> = ctx.scene_tree.share();
88+
89+
// If you don't need the scene, you can also construct free-standing nodes:
90+
let node: Gd<Node3D> = Node3D::new_alloc();
91+
// ...
92+
node.free(); // don't forget to free everything created by new_alloc().
93+
}
94+
95+
// Skip a test that's not yet ready.
96+
#[itest(skip)]
97+
fn not_executed() {
98+
// ...
99+
}
100+
101+
// Focus on a one or a few tests.
102+
// As soon as there is at least one #[itest(focus)], only focused tests are run.
103+
#[itest(focus)]
104+
fn i_need_to_debug_this() {
105+
// ...
106+
}
107+
```
38108

39109
You can run the integration tests like this:
40110

41-
```
111+
```bash
42112
$ ./check.sh itest
43113
```
44114

45115
Just like when compiling the crate, the `GODOT4_BIN` environment variable can be used to supply the path and filename of your Godot executable.
116+
Otherwise, a binary named `godot4` in your PATH is used.
117+
46118

47-
## Formatting
119+
### Formatting
48120

49121
`rustfmt` is used to format code. `check.sh` only warns about formatting issues, but does not fix them. To do that, run:
50122

51-
```
123+
```bash
52124
$ cargo fmt
53125
```
54126

55-
## Clippy
127+
128+
### Clippy
56129

57130
`clippy` is used for additional lint warnings not implemented in `rustc`. This, too, is best run through `check.sh`:
58131

132+
```bash
133+
$ ./check.sh clippy
59134
```
60-
$ check.sh clippy
135+
136+
## Continuous Integration
137+
138+
If you want to have the full CI experience, you can experiment as much as you like on your own gdext fork, before submitting a pull request.
139+
140+
For this, navigate to the file `.github/workflows/full-ci.yml` and change the following lines:
141+
142+
```yml
143+
on:
144+
push:
145+
branches:
146+
- staging
147+
- trying
61148
```
62149
63-
## Real
150+
to:
151+
152+
```yml
153+
on:
154+
push:
155+
```
156+
157+
This runs the entire CI pipeline to run on every push. You can then see the results in the _Actions_ tab in your repository.
158+
159+
Don't forget to undo this before opening a PR! You may want to keep it in a separate commit named "UNDO" or similar.
64160
65-
Certain types in Godot use either a single or double-precision float internally, such as `Vector2`. When using these types we
66-
use the `real` type instead of choosing either `f32` or `f64`. Thus our code is portable between Godot binaries compiled with
67-
`precision=single` or `precision=double`.
161+
162+
## Build configurations
163+
164+
### `real` type
165+
166+
Certain types in Godot use either a single or double-precision float internally, such as `Vector2`.
167+
When working with these types, we use the `real` type instead of choosing either `f32` or `f64`.
168+
As a result, our code is portable between Godot binaries compiled with `precision=single` and `precision=double`.
68169

69170
To run the testing suite with `double-precision` enabled you may add `--double` to a `check.sh` invocation:
70-
```
71-
$ check.sh --double
171+
```bash
172+
$ ./check.sh --double
72173
```
73174

74175
[GitHub issue]: https://github.com/godot-rust/gdext/issues

ReadMe.md

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,44 +28,43 @@ At this point, there is **no** support for Android, iOS or WASM. Contributions a
2828

2929
## Getting started
3030

31-
### Toolchain
32-
33-
You need to have LLVM installed to use `bindgen`, see [the book](https://godot-rust.github.io/book/getting-started/setup.html#llvm) for instructions.
31+
An elaborate tutorial is available [in the book] (still under construction), here is the short version.
3432

3533
To find a version of Godot 4, the library expects either an executable of name `godot4` in the PATH, or an environment variable `GODOT4_BIN`
3634
containing the path to the executable (including filename).
35+
We currently only have a GitHub version, crates.io releases are planned once more of the foundation is ready.
3736

38-
### Project setup
39-
40-
We currently only have a GitHub version, crates.io releases are planned once more of the foundation is ready.
4137
In your Cargo.toml, add:
4238

4339
```toml
44-
[dependencies]
45-
godot = { git = "https://github.com/godot-rust/gdext", branch = "master" }
46-
4740
[lib]
4841
crate-type = ["cdylib"]
42+
43+
[dependencies]
44+
godot = { git = "https://github.com/godot-rust/gdext", branch = "master" }
4945
```
5046
To get the latest changes, you can regularly run a `cargo update` (possibly breaking). Keep your `Cargo.lock` file under version control, so that it's easy to revert updates.
5147

5248
To register the GDExtension library with Godot, you need to create two files relative to your Godot project folder:
5349

54-
1. First, add `res://MyExt.gdextension`, which is the equivalent of `.gdnlib` for GDNative.
55-
50+
1. First, add `res://MyExt.gdextension`, which is the equivalent of `.gdnlib` for GDNative.
51+
5652
The `[configuration]` section should be copied as-is.
5753
The `[libraries]` section should be updated to match the paths of your dynamic Rust libraries.
54+
`{my-ext}` can be replaced with the name of your crate.
5855
```ini
5956
[configuration]
6057
entry_symbol = "gdext_rust_init"
6158

6259
[libraries]
63-
linux.debug.x86_64 = "res://../rust/target/debug/lib{my_ext}.so"
64-
linux.release.x86_64 = "res://../rust/target/release/lib{my_ext}.so"
65-
windows.debug.x86_64 = "res://../rust/target/debug/{my_ext}.dll"
66-
windows.release.x86_64 = "res://../rust/target/release/{my_ext}.dll"
67-
macos.debug = "res://../rust/target/debug/{my_ext}.dylib"
68-
macos.release = "res://../rust/target/release/{my_ext}.dylib"
60+
linux.debug.x86_64 = "res://../rust/target/debug/lib{my-ext}.so"
61+
linux.release.x86_64 = "res://../rust/target/release/lib{my-ext}.so"
62+
windows.debug.x86_64 = "res://../rust/target/debug/{my-ext}.dll"
63+
windows.release.x86_64 = "res://../rust/target/release/{my-ext}.dll"
64+
macos.debug = "res://../rust/target/debug/{my-ext}.dylib"
65+
macos.release = "res://../rust/target/release/{my-ext}.dylib"
66+
macos.debug.arm64 = "res://../rust/target/aarch64-apple-darwin/debug/{my-ext}.dylib"
67+
macos.release.arm64 = "res://../rust/target/aarch64-apple-darwin/release/{my-ext}.dylib"
6968
```
7069
(Note that for exporting your project, you'll need to use paths inside `res://`).
7170

@@ -80,7 +79,7 @@ To register the GDExtension library with Godot, you need to create two files rel
8079
We highly recommend to have a look at a working example in the `examples/dodge-the-creeps` directory.
8180
This integrates a small game with Godot and has all the necessary steps set up.
8281

83-
API documentation can be generated locally using `cargo doc -p godot --no-deps --open`.
82+
API documentation can be generated locally using `./check.sh doc` (use `dok` instead of `doc` to open the page in the browser).
8483

8584
If you need help, join our [Discord] server and ask in the `#help-gdext` channel!
8685

@@ -90,7 +89,7 @@ If you need help, join our [Discord] server and ask in the `#help-gdext` channel
9089
We use the [Mozilla Public License 2.0][mpl]. MPL tries to find a balance between permissive (MIT, Apache, Zlib) and copyleft licenses (GPL, LGPL).
9190

9291
The license provides a lot of freedom: you can use the library commercially and keep your own code closed-source,
93-
i.e. game development is not restricted. The main condition is that if you change godot-rust _itself_, you need to make
92+
i.e. game development is not restricted. The main condition is that if you change godot-rust _itself_, you need to make
9493
those changes available (and only those, no surrounding code).
9594

9695

@@ -104,3 +103,4 @@ Contributions are very welcome! If you want to help out, see [`Contributing.md`]
104103
[Discord]: https://discord.gg/aKUCJ8rJsc
105104
[Mastodon]: https://mastodon.gamedev.place/@GodotRust
106105
[Twitter]: https://twitter.com/GodotRust
106+
[in the book]: https://godot-rust.github.io/book/gdext/intro

examples/dodge-the-creeps/godot/DodgeTheCreeps.gdextension

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ entry_symbol = "gdext_rust_init"
44
[libraries]
55
linux.debug.x86_64 = "res://../../../target/debug/libdodge_the_creeps.so"
66
linux.release.x86_64 = "res://../../../target/release/libdodge_the_creeps.so"
7-
macos.debug = "res://../../../target/debug/libdodge_the_creeps.dylib"
8-
macos.release = "res://../../../target/release/libdodge_the_creeps.dylib"
97
windows.debug.x86_64 = "res://../../../target/debug/dodge_the_creeps.dll"
108
windows.release.x86_64 = "res://../../../target/release/dodge_the_creeps.dll"
9+
macos.debug = "res://../../../target/debug/libdodge_the_creeps.dylib"
10+
macos.release = "res://../../../target/release/libdodge_the_creeps.dylib"
11+
macos.debug.arm64 = "res://../../../target/aarch64-apple-darwin/debug/libdodge_the_creeps.dylib"
12+
macos.release.arm64 = "res://../../../target/aarch64-apple-darwin/release/libdodge_the_creeps.dylib"

itest/godot/itest.gdextension

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
entry_symbol = "itest_init"
33

44
[libraries]
5-
linux.x86_64 = "res://../../target/debug/libitest.so"
6-
windows.x86_64 = "res://../../target/debug/itest.dll"
7-
macos = "res://../../target/debug/libitest.dylib"
5+
linux.debug.x86_64 = "res://../../target/debug/libitest.so"
6+
linux.release.x86_64 = "res://../../target/release/libitest.so"
7+
windows.debug.x86_64 = "res://../../target/debug/itest.dll"
8+
windows.release.x86_64 = "res://../../target/release/itest.dll"
9+
macos.debug = "res://../../target/debug/libitest.dylib"
10+
macos.release = "res://../../target/release/libitest.dylib"
11+
macos.debug.arm64 = "res://../../target/aarch64-apple-darwin/debug/libitest.dylib"
12+
macos.release.arm64 = "res://../../target/aarch64-apple-darwin/release/libitest.dylib"

0 commit comments

Comments
 (0)