Skip to content

Conversation

@versjon
Copy link
Contributor

@versjon versjon commented May 24, 2025

Added boids demo with gdscript and numdot solver, currently numdot version less performant, #48

@Ivorforce
Copy link
Owner

Hi! Sorry for getting back so late.
I've had a quick look. The code looks good to me (although I'll probably toy with it to see if I can't get the performance to be better).

I'd love to merge; however you appear to have quite a lot of changes unrelated to the boids demo in your PR.
I'd like to ask you to remove those changes before I'll merge. Especially, the update of godot-cpp and xtensor are problematic. But also all the whitespace changes are out of scope, making this PR too large.

Do you know how to approach this? I'm happy to lend a hand if you run into trouble :)

@kro-ma
Copy link
Contributor

kro-ma commented Jun 1, 2025

Thanks for the feedback!

EDIT: Actually I managed to revert the changes in the files out of scope, or at least these changes are not shown as changed in this PR anymore.

I tried to undo the changes that happened to the files out of scope, but had no luck so far. Afaik the out of scope changes were only commited in this commit: 76567b9.

Any help or advice to just revert the changes for these files out of scope would be highly appreciated!

You can also contact versjon or me (maxk_2) in Discord.

Copy link
Owner

@Ivorforce Ivorforce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Thank you both again @versjon and @kro-ma for your hard work!

I will merge this PR now, and likely have a look over later. I recall you saying the Godot version is currently faster than the NumDot version, which may make it a great opportunity to improve performance internally :)
If you find anything else you want to improve, feel free to make a follow-up PR!

@Ivorforce Ivorforce merged commit 19f1ea5 into Ivorforce:main Jun 2, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants