zackradisic / aussieplusplus

Programming language from down under
aussieplusplus.vercel.app
609 stars 15 forks source link

Fix range check for ChuckSomeDice() #42

Open bbrk24 opened 2 years ago

bbrk24 commented 2 years ago

Fixes #41

It turns out the error wasn't about floats per se -- ChuckSomeDice(0, 0) would cause the same panic, while ChuckSomeDice(0, 1.5) wouldn't.

vercel[bot] commented 2 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/zackradisic/aussieplusplus/5PXUsgyiMpB5WS3Jc5EiyYuJLdq4
✅ Preview: https://aussieplusplus-git-fork-bbrk24-range-check-fix-zackradisic.vercel.app

bbrk24 commented 2 years ago

Hm, the preview still has the error. I'm not quite sure what's going on.

bbrk24 commented 2 years ago

What's weird is it works locally on my machine. I'm completely new to Rust, so I have no idea how to diagnose the discrepancy.

bbrk24 commented 2 years ago

Yeah I won’t be able to figure out the difference. I’m hopeful it’s just something wrong with the preview.

dspingarn commented 2 years ago

I'm guessing that the preview is still running the master branch somehow, but only @zackradisic would know for sure, since I can't see the vercel configuration...

I was able to run this change on my local machine and can see that changing from > to >= works correctly in making sure the proper error message is printed if the range is 0,0! ...I deployed your change on my own instance of vercel here -> https://aussieplusplus-4y3o5ei6a-dspingarn.vercel.app/

dspingarn commented 2 years ago

actually, I know what the issue is, @bbrk24 You need to run make wasm to make sure the live demo that's generated contains your new code change. Right now, your logic change is just contained in the aussie++ code, so the preview won't reflect that

bbrk24 commented 2 years ago

You need to run make wasm to make sure the live demo that's generated contains your new code change

Huh. I figured such a step would be done automatically or wouldn't touch checked-in files. I tried it just now, locally, and I got this linker error:

'emcc.bat' is not recognized as an internal or external command, operable program or batch file.

I'm not entirely sure why a cmd error came up when I'm using bash, and the solutions I'm finding online don't seem to work either.

dspingarn commented 2 years ago

emcc is the webassembly compiler ...this project definitely needs a better "first time setup" guide anyway. You can follow https://emscripten.org/docs/getting_started/downloads.html to figure out how to configure the emsdk on your machine

dspingarn commented 2 years ago

from there, when you run make wasm, if it's successful, you should see some files in the site folder get updated. cd into the site folder, and that's a node project of some kind... you can install and run it using yarn (which, incidentally, I believe the yarn build task is using next.js in the background, but that's not important) you can do yarn build and then yarn run start to have it running on localhost:3000