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

Does FLOAT.FROMBOOLEAN use the correct stack? #1

Open
rev111 opened this issue Jul 11, 2020 · 1 comment
Open

Does FLOAT.FROMBOOLEAN use the correct stack? #1

rev111 opened this issue Jul 11, 2020 · 1 comment

Comments

@rev111
Copy link

rev111 commented Jul 11, 2020

I try to convert the result of a boolean operation into a float.
This push program

1000.0 500.0 FLOAT.> FLOAT.FROMBOOLEAN

does not convert the boolean.
However

1000.0 500.0 FLOAT.> 0.0 FLOAT.FROMBOOLEAN

works: The float stack then contains 0 and 1.
I think this is because the stack used for that instruction in line 723 is the float stack:

this[ 'FLOAT.FROMBOOLEAN' ] = new pushInstruction( this.floatStack, pushInstructionFromBoolean );

That stack is empty, and later on, pushInstructionFromBoolean in line 203 is ignoring the instruction.
Question:
Is this correct or a bug? I suspect the boolStack should be used here, but I have too little experience with Push and interpreters to be sure.
If I'm wrong, what would be the correct way to convert a boolean result into a float (0.0 or 1.0)?
EDIT: It seems hackish to me to push the 0 on the stack to make this work. Is there a better way?
EDIT2: I think I found it: I changed line 204 from

if( inStack.length > 0 ) {

to

if( inInterpreter.boolStack.length > 0 ) {

Now the boolean stack is checked for content and the float stack contains the converted boolean.
The first push program above works now, hope this is a fix to the intended purpose.

@jonklein
Copy link
Owner

Yes, your fix looks correct, and it looks like the other "fromBoolean" functions suffer from the same issue. Feel free to put in a pull request with your fix!

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 a pull request may close this issue.

2 participants