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

[Semio] Scratch Update/Fixes #488

Merged
merged 18 commits into from
Oct 21, 2024
Merged

Conversation

chrismbirmingham
Copy link
Contributor

This is an updated PR for scratch support that is ready to be merged onto the existing master branch. To fix the issue referenced in #461 (comment) use this version of itch: https://github.com/chrismbirmingham/itch#34

@tcorbly
Copy link
Member

tcorbly commented Oct 19, 2024

There appears to be some issue when building:

  1. You seem to need to run git submodule update --init twice in order for it to fully update all the submodules
  2. When running yarn run build-deps eventually the below error occurs
CMake Error: The source directory "/home/tcorbly/Simulator/dependencies/kipr-scratch/libwallaby" does not appear to contain CMakeLists.txt.
Specify --help for usage, or press the help button on the CMake GUI.
Failed to configure libwallaby.
Traceback (most recent call last):
  File "/home/tcorbly/Simulator/dependencies/build.py", line 233, in <module>
    subprocess.run(
  File "/usr/lib/python3.10/subprocess.py", line 526, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['python3.10', PosixPath('/home/tcorbly/Simulator/dependencies/kipr-scratch/build.py')]' returned non-zero exit status 1.

The fix seems to be to go into dependencies/kipr-scratch/libwallaby and run git submodule update --init

@tcorbly
Copy link
Member

tcorbly commented Oct 19, 2024

It seems the issue here:
#461 (comment)
Is that the show variable block shows every time a new step is executed

@tcorbly
Copy link
Member

tcorbly commented Oct 19, 2024

Last issue that I've seen is that change VARIABLE by NUMBER does not change the variable at all.

Copy link
Member

@tcorbly tcorbly left a comment

Choose a reason for hiding this comment

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

I also noticed that the work for the LMS PR is included in this one. It may need to be removed or the LMS PR will need to merge first.

dependencies/build.py Show resolved Hide resolved
@chrismbirmingham
Copy link
Contributor Author

Thanks for the feedback. I've updated the readme instructions to reflect the need to run git submodules init twice, and pushed a fix for the change VARIABLE by NUMBER bug. You are correct, Show Variable does run on every step, if you want it to run once you should show and the stop show right after. If this is not the desired behavior let me know, I work on alternatives.

@chrismbirmingham
Copy link
Contributor Author

As for the LMS work, I had intended that to merge first as it seemed like a simpler PR - but if this is done first I can pull those commits out to make this a clean PR.

@tcorbly
Copy link
Member

tcorbly commented Oct 21, 2024

I think you are correct that the showing of a variable after each command is the correct behavior since the hide variable command exists. I don't see any other issues though so I think this PR is ready to merge!

@tcorbly tcorbly merged commit 6abed8a into kipr:master Oct 21, 2024
1 check 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