search

Team-CC-Corp / JVML-JIT

A continuation of JVML which JITs Java bytecode to Lua bytecode rather than interpreting.
MIT License
30 stars 4 forks source link

TerminalInputStream, system.in #38

Closed ghost closed 10 years ago

ghost commented 10 years ago

For some reason, if I wrap TerminalInputStream in a BufferedInputStream it does not work as expected. Other than that, System.in works perfectly.

ghost commented 10 years ago

Honestly, I did not look into the buffered thing much... As far as the TerminalInputStream being wrapped, I mean... wrapping it broke the functionality so.. IDK.

ghost commented 10 years ago

Forgot to mention, I fixed Computer.read()

ElvishJerricco commented 10 years ago

Well what's the issue when you try to use the BufferedInputStream?

ElvishJerricco commented 10 years ago

Also, minor thing, don't change code from 

if() {

To

if()
{

=P It's rather unprofessional for a project to have different styles for stuff like that throughout, and all of it is already in the first style.

ghost commented 10 years ago

http://imgur.com/UKzdy1X

Looking into it. That was when I removed the synchronization. The thing is: BufferedInputStream has a ton of concurrency stuff.. so..

Also, sorry about that :P

ElvishJerricco commented 10 years ago

It looks like TerminalInputStream.read() isn't resetting index when it resets current. Could that be the issue?

ghost commented 10 years ago

It should be... If that were the issue it wouldn't work without the BufferedInputStream...

ElvishJerricco commented 10 years ago

I think the fact that your test only does one read() call is the reason we're not seeing an issue without the buffered stream. If it were doing more than one, I bet there'd be a problem.

ElvishJerricco commented 10 years ago

Oh wait you have that in an infinite loop... Shouldn't that prevent the other tests from running? Anyway yea fix the index thing and see if buffered stream works.

ghost commented 10 years ago

I didn't add my test to the others... As for the index thing, it isn't broken?

ghost commented 10 years ago

Oh, I'm sorry, I'm an idiot. I forgot I changed it...

Fixing it...

ElvishJerricco commented 10 years ago

So it's working now?

ghost commented 10 years ago

Not with a BufferedInputStream...

Also, do you want me to fix my formatting derps?

ElvishJerricco commented 10 years ago

So wait are you still getting that array index error?

It'd be appreciated =P

ghost commented 10 years ago

No. It works if I dont use a BufferedInputStream.

On it :P

ghost commented 10 years ago

Okay, what about BufferedInputStream? I didn't do anything to fix it...

ElvishJerricco commented 10 years ago

This shouldn't have been merged. It doesn't currently work.

Yevano commented 10 years ago

It doesn't? I thought there was just a problem with the BIS not working. And that was removed.

ghost commented 10 years ago

Yeah, what doesn't work?

The BIS?

ElvishJerricco commented 10 years ago

Well I spoke a bit soon. It didn't work, but the issue was just an import of java.io.BufferedInputSream, which didn't exist. So it couldn't compile. Then there's a couple more minor issues that I just changed. For instance the test should use System.out, not System.err, and it should read one line instead of indefinitely.

There was also a problem with the new line character. The input stream didn't append it to Computer.read(), which lead to an issue where entering a blank string would cause an array index out of bounds exception. And now the test is capable of reading just one line.

ghost commented 10 years ago

Ah... Yeah, the err was just so I could see it in red :P Uhm...

Yevano commented 10 years ago

Ah sorry. I should have tested before merging.

ElvishJerricco commented 10 years ago

Got it! =P There were a couple weird issues that needed solving for BufferedInputStream to work but it's good now.

ghost commented 10 years ago

Cool! Glad to hear it!