Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a basic GUI to Hypermine powered by yakui #395

Merged
merged 3 commits into from
May 10, 2024
Merged

Conversation

patowen
Copy link
Collaborator

@patowen patowen commented May 6, 2024

This PR adds a GUI with the following features:

  • A central cursor
  • Some indicator (likely textual) of the block you have selected
  • The ability to turn it on and off (for screenshot purposes) with F1

image

@patowen
Copy link
Collaborator Author

patowen commented May 6, 2024

Draft status: This version currently crashes intermittently when switching materials with a Vulkan validation error. Based on my own testing, I believe that this can be resolved when SecondHalfGames/yakui#155 is merged, and this PR points to the new branch. See comment thread of #391 for details.

Otherwise, I think it's ready for review.

EDIT: Now that the PR has been merged, I'll upgrade this to a regular PR, although I am aware that some changes will still be needed. Thanks for reviewing!

@patowen patowen requested a review from Ralith May 6, 2024 00:26
@patowen patowen changed the title Add a basic UI to Hypermine powered by yakui Add a basic GUI to Hypermine powered by yakui May 6, 2024
client/src/graphics/draw.rs Outdated Show resolved Hide resolved
client/src/graphics/draw.rs Outdated Show resolved Hide resolved
client/src/graphics/gui.rs Outdated Show resolved Hide resolved
@patowen patowen marked this pull request as ready for review May 6, 2024 05:24
@patowen
Copy link
Collaborator Author

patowen commented May 7, 2024

While testing this, I encountered a (potentially rare) warning log after I closed the application saying the following:
WARN sendmsg error: Os { code: 10093, kind: Uncategorized, message: "Either the application has not called WSAStartup, or WSAStartup failed." }, Transmit: { destination: [::1]:60027, src_ip: None, enc: None, len: 202, segment_size: None }

This seems to be a log from quinn-udp::log_sendmsg_error. I'm not sure whether it's important, since that might mean that Quinn tried to send a message after something was already finalized.

My cargo.lock has quinn at version 0.10.0 with quinn-udp at version 0.4.1 and quinn-proto at 0.10.5.

EDIT: Now that Quinn has been upgraded, this comment is likely irrelevant now. Thanks for upgrading it!

@patowen
Copy link
Collaborator Author

patowen commented May 9, 2024

I had renamed the gui module to gui_state because it only contained a struct called GuiState and its implementation. However, I'm starting to prefer the name gui again because it seems like a good place to put future things, such as custom widgets etc.

I expect this code might see a fair bit of churn in future PRs just due to the uncertainty of how all the UI elements will work together.

@Ralith
Copy link
Owner

Ralith commented May 9, 2024

EDIT: Now that Quinn has been upgraded, this comment is likely irrelevant now. Thanks for upgrading it!

I don't think we changed anything that would be relevant to that error, FWIW. Sounds like something called WSACleanup unexpectedly, so if we ever see it reproducibly, that might be something to try to catch in a debugger. Out of scope of this PR, though.

client/src/graphics/base.rs Outdated Show resolved Hide resolved
client/src/graphics/draw.rs Outdated Show resolved Hide resolved
client/src/graphics/gui.rs Outdated Show resolved Hide resolved
client/src/graphics/window.rs Outdated Show resolved Hide resolved
@Ralith Ralith merged commit 538b549 into Ralith:master May 10, 2024
4 checks passed
@patowen patowen deleted the yakui branch May 10, 2024 01:35
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.

2 participants