Core: Fix Generate's slot parsing to default unknown slot names to file name #3795
Conversation
github-actions
bot
added
affects: core
Exempt-Medic
added
the
is: bug/fix
| if path == args.weights_file_path: # if name came from the weights file, just use base player name | ||
| erargs.name[player] = f"Player{player}" | ||
| elif not erargs.name[player]: # if name was not specified, generate it from filename | ||
| elif player not in erargs.name: # if name was not specified, generate it from filename | ||
| erargs.name[player] = os.path.splitext(os.path.split(path)[-1])[0] | ||
| erargs.name[player] = handle_name(erargs.name[player], player, name_counter) |
There was a problem hiding this comment.
Unsure if you'd want to add this to this PR, but this does technically change the fact that previously you could name yourself false (which would get changed to the yaml name) and now you cannot name yourself anything boolean in yaml. Could add this to fix it, but maybe there's a better way?
| erargs.name[player] = handle_name(erargs.name[player], player, name_counter) | |
| erargs.name[player] = handle_name(str(erargs.name[player]), player, name_counter) |
There was a problem hiding this comment.
what is expected / what currently works? if you name yourself "false" it currently ignores the name and runs through this code to get the filename instead?
There was a problem hiding this comment.
If you name yourself false (no quotation marks) currently it ends up actually hitting the yaml name rename line because your name evaluates as, well, False. This is clearly unintended, but the name does work just fine on WebHost, so I guess it converts to a string somewhere. I think it would make sense to do the same here so that name: false, name: on etc. will gen instead of crashing
What is this fixing or adding?
make Generate handle slots without names defined better; fix existing code that seemed to want to handle missing names but was no longer functioning correctly
if there is a better solution than what the existing code was trying to do then i differ to that
How was this tested?
https://discord.com/channels/731205301247803413/1273345114198118502
tested with the original provided yaml and with a copy that moved the slot without a name to the bottom of the yaml and confirmed with pdb that the args.name got created properly and defaulted to file name when name was missing
also tested with two slots missing name in the same yaml and saw it throw an error mentioning the duplicate names, not sure if that's the best error handling but it works enough for me
If this makes graphical changes, please attach screenshots.