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

Fixed the external.invoke by @pierredavidbelanger #47

Merged
merged 3 commits into from
Dec 29, 2022

Conversation

Erquint
Copy link
Owner

@Erquint Erquint commented Dec 29, 2022

Fixes #19

@Erquint
Copy link
Owner Author

Erquint commented Dec 29, 2022

@pierredavidbelanger Hello. First of all I'm sorry for all the mess.
Since the maintainer of the original repo is 2 years absent, I'm trying to accept community contributions here and have them released automatically.
I finally ironed out most of the GitHub Workflows kinks to publish built releases upon PR merge commits.
I'm ready to investigate you contribution now.

In the original PR I said something about hardcoding an OS and that was wrong of me — seems I confused some of your changes with parts of another PRs changes by another person.

I wanna try building your latest change, but before I do that, can you confirm that the latest build in the releases section still has an issue to fix? Give me a signoff and I'll merge this PR here to trigger a build for testing whether your change fixes it for all platforms.

@pierredavidbelanger
Copy link

Oh! Nice of you of resurecting this tool. Ill help.

As soon as I have a couple minute in front of the PC ill try your latest release.

Then I will clone your fork instead of the upstream version so I can tryt to help, it will not be much though, I have no clue about what i am doing when working with Rust yet.

@Erquint
Copy link
Owner Author

Erquint commented Dec 29, 2022

Your fix branch is already part of this here PR I opened myself, so you don't really have to re-fork necessarily.

I realized I accidentally made it package extra crud from the compilation directory along with the executable with the release. Afraid of touching the build script right now, during this PR. Hopefully that's not a problem. You can discard everything aside from the executable.

Take your time and merry holidays. 😄

@pierredavidbelanger
Copy link

Ok, I can confirm , your latest build for linux throws a lot of javascript error, and also has no widgets in the UI

image

I may be worth to note, I am starting the app with custom gamelog and soundpack options (both paths exists):

./soundsense-rs -l ../thelongnight/gamelog.txt -p packs

@Erquint
Copy link
Owner Author

Erquint commented Dec 29, 2022

That's how it was on the original repo, I have to assume.
Well, let's try merging now. A new release should be posted in about 10 minutes or less.

@Erquint Erquint merged commit 3d4d6ec into Erquint:master Dec 29, 2022
@Erquint
Copy link
Owner Author

Erquint commented Dec 29, 2022

Gosh. Something went wrong with the workflow run.
I'm very sorry. Need to understand how to fix that first in order to be able to build a release.

@Erquint
Copy link
Owner Author

Erquint commented Dec 29, 2022

I think the permissions are restricted for fork PRs.
I could only test the build script with my own PRs so far.

@Erquint
Copy link
Owner Author

Erquint commented Dec 29, 2022

Couldn't really re-run the workflow after changing its permissions on this already merged PR.
Triggered a release with another bogus PR.
Time to test the new binaries.

@Erquint
Copy link
Owner Author

Erquint commented Dec 29, 2022

Seems to work fine on Windows here.

@Erquint
Copy link
Owner Author

Erquint commented Dec 29, 2022

By the way… 😅

I have no clue about what i am doing when working with Rust yet.

Me neither!

I'm not actually getting into developing this project but just want to maintain it in order to allow community contributions — like yours.
Don't even have the Rust development environment set up locally. That should explain why I'm relying on remote building so much.

@Erquint
Copy link
Owner Author

Erquint commented Dec 29, 2022

Had my friend @Elerphore test on Ubuntu 22.04.1 LTS x86_64 and he reported no issues.
Playback functions. Sliders slide. No errors in the log.

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.

"ReferenceError: Can't find variable: external" when interacting with any GUI element
2 participants