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

Added a command that modifies the saved replay #25

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

skyborgff
Copy link
Member

Modifications include making the goals 0 - 0 and hiding the markers
on the timeline. This makes so we can stream replays without spoilers.

Unfortunately I run BLACK on the project without noticing until it was too late...

Modifications include making the goals 0 - 0 and hiding the markers
on the timeline. This makes so we can stream replays without spoilers.

Unfortunately I run BLACK on the project without noticing until it was too late...
@tarehart
Copy link
Contributor

How hard would it be to remake the PR without the changes from BLACK?

@skyborgff
Copy link
Member Author

It depends on how the rollback feature is implemented on pycharm. I'll have a look and in last resort, I'll change it manually

@skyborgff
Copy link
Member Author

@tarehart should be ok now, i didnt had time to test if it still works as i have to leave now

Copy link
Collaborator

@NicEastvillage NicEastvillage left a comment

Choose a reason for hiding this comment

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

This is a great idea but I don't think it is implemented very well. It also seems like there are some mistakes.

I especially don't like that you pass the working_dir to ReplayMonitor, as the replay monitor is meant to return the replay to the run_match() function. In other words, I think you should call the anonymize function from the run_match() function https://github.com/RLBot/AutoLeaguePlay/blob/master/autoleagueplay/run_matches.py#L43-L48. That will also allow you to name the replay something relevant.

settings.json
/autoleagueplay/bots/
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would not recommend using the autoleagueplay folder as your working directory for league play (I am assuming that is why you had to add this). That will get annoying since ALP creates a lot of files. Either use league/ which is already ignored or a totally different folder.



def upload_to_calculated_gg(replay_path: Path):
with open(replay_path, 'rb') as f:
response = requests.post('https://calculated.gg/api/upload', files={'replays': f})
print(f'upload response to {replay_path.name}: {response}')

def anonymize_replay(replay_path: Path, working_dir: WorkingDir):
replay_name = str(os.path.basename(str(replay_path)).split('.')[0])
anonym_replay_path = working_dir.replays / str(replay_name + '.replay')
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about remaning the replay file to something reasonable? Like "Beast from the East vs Skybot"? This means you will have to call the anonymize_play() function from the run_match() function https://github.com/RLBot/AutoLeaguePlay/blob/master/autoleagueplay/run_matches.py#L43-L48 where your other match info is available. Calling it from there actually makes a lot more sense if you ask me. You would also avoid passing the working_dir to ReplayMonitor for instance.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to do that. i can even rename the in-game name, but i couldnt from replay.py
I'll work on those changes



def upload_to_calculated_gg(replay_path: Path):
with open(replay_path, 'rb') as f:
response = requests.post('https://calculated.gg/api/upload', files={'replays': f})
print(f'upload response to {replay_path.name}: {response}')

def anonymize_replay(replay_path: Path, working_dir: WorkingDir):
replay_name = str(os.path.basename(str(replay_path)).split('.')[0])
Copy link
Collaborator

Choose a reason for hiding this comment

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

You already have the Path object, so you can just do

Suggested change
replay_name = str(os.path.basename(str(replay_path)).split('.')[0])
replay_name = replay_path.stem

autoleagueplay/replays.py Show resolved Hide resolved
@@ -79,6 +79,7 @@ def on_tick(self, tick: TrainingTickPacket) -> Optional[Grade]:
time.sleep(1) # Give time for replay_monitor to register replay and for RL to load main menu
if self.replay_monitor.replay_id or self.replay_monitor.replay_preference == ReplayPreference.IGNORE_REPLAY:
self.replay_monitor.stop_monitoring()
self.replay_monitor.anonymize_replay()
Copy link
Collaborator

Choose a reason for hiding this comment

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

The ReplayMonitor does not have this function, anonymize_replay(). There is a similarly named function replays.py file though. You also try to make this call in line 92 and 97.

try:
replay['content']['body']['marks'] = []
except:
pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious, does all these try-excepts has to be done individually?

Copy link
Member Author

Choose a reason for hiding this comment

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

there may be a better way, but if one fails, i want it to keep going to the next ( for example, if team 0 doesn't score, that field doesn't exist)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then this is probably fine.

@skyborgff skyborgff added this to In progress in Full Automation Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Full Automation
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants