cocacola-lab / MineLand

Simulating Large-Scale Multi-Agent Interactions with Limited Multimodal Senses and Physical Needs
MIT License
56 stars 9 forks source link

Error comparing byte type and string in MineflayerManager listen_stdout and listen_stderr #21

Closed jbarrerobuch closed 2 months ago

jbarrerobuch commented 2 months ago
def listen_stdout(self) :
        print('Start to listen stdout.')
        while True and self.process:
            output = self.process.stdout.readline()
            if output == '':
                continue
            if any(substr in output for substr in self.version_not_match_filter):
                print_error("Warning: Node.js version is not matched! Please use v18.18.2!")

            if 'started' in output:
                self.is_running = True
            print(output, end='')
        print("Mineflayer is end.")

def listen_stderr(self) :
        while True and self.process:
            output = self.process.stderr.readline()

            if output == '':
                continue
            if any(substr in output for substr in self.output_filter):
                continue
            if output.isspace():
                continue
            if any(substr in output for substr in self.version_not_match_filter):
                print_error("Warning: Node.js version is not matched! Please use v18.18.2!")

            print(output, end='')

The if statements in the while loop need to be nested to catch the different values of the output. And the condition:

if any(substr in output for substr in self.version_not_match_filter)

this returns an error because output is of type byte by definition (line 67 or line 81) and being compared to string.

output = self.process.stdout.readline() output = self.process.stderr.readline()

I suggest as fix converting the value of output to string before comparing.

def listen_stdout(self) :
        print('Start to listen stdout.')
        while True and self.process:
            output = self.process.stdout.readline()
            output_str = output.decode('utf-8')

            if output == '':
                continue
            elif any(substr in output_str for substr in self.version_not_match_filter):
                print_error("Warning: Node.js version is not matched! Please use v18.18.2!")

            elif 'started' in output_str:
                self.is_running = True

            if output_str == '':
                print("...", end='\r')
            else:
                print(f"\n {output_str}")

        print("Mineflayer is end.")

    def listen_stderr(self) :
        while True and self.process:
            output = self.process.stderr.readline()
            output_str = output.decode('utf-8')

            if output == '':
                continue
            elif any(substr in output_str for substr in self.output_filter):
                continue
            elif output.isspace():
                continue
            elif any(substr in output_str for substr in self.version_not_match_filter):
                print_error("Warning: Node.js version is not matched! Please use v18.18.2!")

            if output_str == '':
                print("...", end='\r')
            else:
                print(output_str, end='')

also adding a conditional in the print statement in the end to avoid flooding the console. However the value of output I get is always "" so there must be another issue somewhere.

YXHXianYu commented 2 months ago

Thank you for your feedback!

Issue 1: The value of output

I just tested the code, and in my environment, the type is not an issue. output = self.process.stdout.readline() returns a str, not bytes. When creating the process, I added the parameter text=True , which typically makes readline() return a str directly without needing any conversion.

Issue 2: if statement

Your suggestion makes sense. Using nested if statements would make the logic less prone to errors. I'll submit a PR with this fix shortly.

Issue 3: Flooding the console

In my use case, I didn’t encounter this issue because I checked both output == '' and output.isspace().

Considering the first issue, I suspect the difference might come from our environments. I tested this on both Windows 11 and Arch Linux with Python3.11. If possible, could you provide more detailed information about your environment?

Thank you again for your feedback!

YXHXianYu commented 2 months ago

Refer to #16 this issue, I found the differences.

I fixed these issues in the lastest PR.

However, since I can't reproduce these issues, I can't be certain if the problem is truly resolved. If you have any further questions, feel free to ask.

Hope this helps you.

YXHXianYu commented 2 months ago

This issue is most likely caused by a low java version. You can use java -version to check it.

If the above method doesn't solve the issue. You can modify the command in server_manager.py directly to check the environment(https://github.com/cocacola-lab/MineLand/blob/main/mineland/sim/server_manager.py#L92)

From:

command = f"java -Xmx8G -jar fabric-server-mc.1.19-loader.0.14.18-launcher.0.11.2.jar nogui"
args = shlex.split(command)

To:

command = f"java -version"
args = shlex.split(command)

This method will check the version that MineLand is actually using.

jbarrerobuch commented 2 months ago

ohh sorry I just deleted the comment. I run it again and the exception didn't raise again, so I thought it could be because I did a new clone in a new folder and I forgot to install the java dependencies npm ci

Thanks a lot for answering my doubts. I'm really interested in understanding you project, so If you don't mind I'll open some questions :)

YXHXianYu commented 2 months ago

ohh sorry I just deleted the comment. I run it again and the exception didn't raise again, so I thought it could be because I did a new clone in a new folder and I forgot to install the java dependencies npm ci

Thanks a lot for answering my doubts. I'm really interested in understanding you project, so If you don't mind I'll open some questions :)

Thanks for your interest. Feel free to open any questions.

Just a heads-up: at the moment, our main focus isn't on MineLand. (though we might return to add new features in the future) So, for some complex issues, I might not respond immediately, but I'll do my best to get back to you as soon as possible. You can also submit PR directly (if you want), I'll review it.

jbarrerobuch commented 2 months ago

Awesome! I'm bit noob on this topic, i'll try my best.

Not sure why but it is getting freezed here. This is what happened before and I could fix it with the code above. running just the validate_install_simulator.py

image

YXHXianYu commented 2 months ago

It seems to be caused by insufficient compute performance. MineLand requires at least 4G of memory. We suggest using MineLand in a home computer (or better).

I tested MineLand in a 4G Tencent Cloud server and it works properly.

YXHXianYu commented 2 months ago

MineLand needs to launch a Minecraft Server, so it needs some computing resource. 😢

jbarrerobuch commented 2 months ago

Ok I left it running and it took some time but it was completed. It was simulating but the output was empty so nothing was printed in the console.

Thanks for your attention. :)