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

ISS-115: Reorganising demo buttons into tabs #117

Merged
merged 4 commits into from
Feb 1, 2025

Conversation

Alphish
Copy link
Owner

@Alphish Alphish commented Jan 27, 2025

Addressing issue #115

The demos are now divided into categories, and selecting a category in the starting room will show its corresponding demos. This should allow adding more demos for a while, without the starting room getting overcrowded.

@Alphish Alphish added the feature ✨ For feature requests and implementations label Jan 27, 2025
@Alphish Alphish self-assigned this Jan 27, 2025
Copy link

@Mercerenies Mercerenies left a comment

Choose a reason for hiding this comment

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

Looks good! Now that I've had a chance to run the actual demo, I should note that a couple of the file read/write tests dump some error output to the console. They still pass, so it's not a big deal, but if there's an easy way to get rid of that you might consider it.

ERROR!!! :: Failed to open file: testing/this_file_does_not_exist.txt for buffer_load()
ERROR!!! :: Failed to open file: testing/this_file_does_not_exist.txt for buffer_load()

// Inherit the parent event
event_inherited();

on_click = method({ target_room, category }, function() {

Choose a reason for hiding this comment

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

I learn so many neat GML tricks from reading your code. My way of making pseudo-closures is so much more obnoxiously verbose than this

@Alphish
Copy link
Owner Author

Alphish commented Feb 1, 2025

I should note that a couple of the file read/write tests dump some error output to the console. They still pass, so it's not a big deal, but if there's an easy way to get rid of that you might consider it.

ERROR!!! :: Failed to open file: testing/this_file_does_not_exist.txt for buffer_load()
ERROR!!! :: Failed to open file: testing/this_file_does_not_exist.txt for buffer_load()

This comes from buffer_load within file_read_all_text
It appears once for testing that file_read_all_text returns undefined for non-available file, and another time for similar json_load test.

I attempt to read a buffer from potentially non-existent file as opposed to checking if file exists beforehand, because:

  1. the file might disappear between the check and the loading
  2. the file might exist but not be available for other reasons (like, I dunno, security)

Thus, I don't think I can get rid of these messages, if I want to keep the "unable to load buffer for whatever reason" path tested.

Glad the rest of the code looks good to go. ^^

@Mercerenies
Copy link

I figured as much. And I agree that preventing the race condition wrt the file existing is more important than getting rid of a rogue line of unit test output. Just thought I'd bring it up in case you weren't aware.

@Alphish Alphish changed the base branch from main to release/24.11.0 February 1, 2025 20:42
@Alphish Alphish merged commit e46223f into release/24.11.0 Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ✨ For feature requests and implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants