Skip to content

Conversation

@naderzare
Copy link
Member

No description provided.

@naderzare naderzare requested a review from SK2iP September 30, 2024 20:59
@SK2iP
Copy link
Member

SK2iP commented Oct 1, 2024

I checked it, and it was great, but I had a misunderstanding about why sometimes state.world_model.stopped_cycle was 0 and other times it was 55. It would be great if you could explain this to me.

directory = os.path.dirname(log_file)
if directory and not os.path.exists(directory):
os.makedirs(directory)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to add a variable that allows users to decide whether the previous log file should be saved or removed. I mean something like this :

        if os.path.exists(log_dir):
            timestamp = datetime.now().strftime("%Y-%m-%d_%H-%M-%S")
            new_log_dir = f"debug/player_logs_{timestamp}"
            shutil.move(log_dir, new_log_dir)

debug=false

# help function
usage() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add -d command to usage function so can people find the -d command

start-team.sh Outdated
#!/bin/bash

# remove logs directory
rm -rf logs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base on what i said in the logger_utils i think you should change here if we want to add that feature

@naderzare naderzare requested a review from SK2iP October 2, 2024 00:19
@naderzare
Copy link
Member Author

@SK2iP Thank you!
It was really good review
please check the thrift as well.

@naderzare naderzare merged commit c15f6ff into main Oct 3, 2024
@naderzare naderzare deleted the improve_logger_and_debug branch November 17, 2024 19:11
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.

3 participants