Skip to content

Godot Blackholio completion#5030

Open
lisandroct wants to merge 11 commits into
masterfrom
lisandro/godot-blackholio-completion
Open

Godot Blackholio completion#5030
lisandroct wants to merge 11 commits into
masterfrom
lisandro/godot-blackholio-completion

Conversation

@lisandroct
Copy link
Copy Markdown
Contributor

Description of Changes

  • Completes the Blackholio demo with username selection, leader board, split mechanic, improved visuals...
  • Add Godot tests

API and ABI breaking changes

  • Nothing

Expected complexity level and risk

  1. It just updates the Blackholio demo project

Testing

  • Play the updated demo
  • Run the tests locally

@lisandroct lisandroct marked this pull request as draft May 15, 2026 14:57
@lisandroct lisandroct force-pushed the lisandro/godot-blackholio-completion branch from d9a4371 to 0c10ba6 Compare May 15, 2026 15:10
@lisandroct lisandroct force-pushed the lisandro/godot-blackholio-completion branch 2 times, most recently from af82eb5 to c2f7cb9 Compare May 22, 2026 19:35
@lisandroct lisandroct marked this pull request as ready for review May 22, 2026 19:37
@lisandroct lisandroct force-pushed the lisandro/godot-blackholio-completion branch from c2f7cb9 to 562bb5d Compare May 28, 2026 16:28
@lisandroct lisandroct force-pushed the lisandro/godot-blackholio-completion branch from 562bb5d to 32fae12 Compare May 28, 2026 16:32
@bfops bfops requested a review from rekhoff May 28, 2026 17:48
<PackageOutputPath>bin~/$(Configuration)/</PackageOutputPath>
<BaseIntermediateOutputPath>obj~/godot/</BaseIntermediateOutputPath>
<IntermediateOutputPath>obj~/godot/$(Configuration)/$(TargetFramework)/</IntermediateOutputPath>
<UseLocalBsatnRuntime Condition="'$(UseLocalBsatnRuntime)' == ''">false</UseLocalBsatnRuntime>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we determined that this didn't actually fix the flakiness, is that right?

Comment thread .github/workflows/ci.yml
permissions:
contents: read
runs-on: spacetimedb-new-runner-2
timeout-minutes: 30
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

how long does a typical successful run take?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FWIW we usually only add this when we've had issues with a job hanging

Comment thread .github/workflows/ci.yml
Comment on lines +720 to +721
# Run cheap .NET setup first. If that fails, no need to run expensive Godot tests.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think this comment makes any sense? the dotnet setup is required for tests, isn't it?

Suggested change
# Run cheap .NET setup first. If that fails, no need to run expensive Godot tests.

Comment thread .github/workflows/ci.yml

- name: Build Godot .NET project
working-directory: demo/Blackholio/client-godot
run: dotnet build --no-restore -v normal blackholio.csproj
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why -v normal?

Comment thread .github/workflows/ci.yml
- name: Publish godot-tests module to SpacetimeDB
working-directory: demo/Blackholio/server-rust
run: |
spacetime logout && spacetime login --server-issued-login local
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: I think the logout part is redundant now. 1) because it's in an ephemeral runner and 2) because we recently made spacetime login automatically do the logout if needed

Suggested change
spacetime logout && spacetime login --server-issued-login local
spacetime login --server-issued-login local

Copy link
Copy Markdown
Collaborator

@bfops bfops left a comment

Choose a reason for hiding this comment

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

The CI part looks pretty good to me, minus a few small comments. It looks like the CI is failing, though.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants