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.

Sign up for GitHub

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

Core: Fix Generate's slot parsing to default unknown slot names to file name #3795

Merged
merged 3 commits into from
Sep 8, 2024
Merged

Core: Fix Generate's slot parsing to default unknown slot names to file name #3795

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e384736
make Generate handle slots without names defined better
qwint Aug 14, 2024
226d34e
set name dict before loop so we don't have to check for its existence…
qwint Sep 8, 2024
1405306
move setter so it's more obvious why
qwint Sep 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Generate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ def main(args=None) -> Tuple[argparse.Namespace, int]:
erargs.outputpath = args.outputpath
erargs.skip_prog_balancing = args.skip_prog_balancing
erargs.skip_output = args.skip_output
erargs.name = {}

settings_cache: Dict[str, Tuple[argparse.Namespace, ...]] = \
{fname: (tuple(roll_settings(yaml, args.plando) for yaml in yamls) if args.sameoptions else None)
Expand Down Expand Up @@ -202,7 +203,7 @@ def main(args=None) -> Tuple[argparse.Namespace, int]:

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)
Copy link
Member

Choose a reason for hiding this comment

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

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?

Suggested change
erargs.name[player] = handle_name(erargs.name[player], player, name_counter)
erargs.name[player] = handle_name(str(erargs.name[player]), player, name_counter)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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


Expand Down
Loading