Spoiler: Display all precollected items in the Spoiler Log #2928
Conversation
github-actions
bot
added
affects: core
PoryGone
added
the
is: enhancement
BaseClasses.py
Outdated
| for item in chain.from_iterable(self.multiworld.precollected_items.values())] | ||
| if precollected_items: | ||
| outfile.write('\n\nPrecollected Items:\n\n') | ||
| outfile.write('\n'.join([item for item in precollected_items])) |
There was a problem hiding this comment.
i think i'd prefer wrapping this in a counter and showing the count with the item rather than just repeating the item count times, but i'm willing to defer if others disagree.
There was a problem hiding this comment.
In this context, I prefer listing them individually, stylistically matching the Locations section to which it is more akin, rather than matching the options output above.
There was a problem hiding this comment.
i was thinking something like
Five Rings: 1 (PoryGone_SA2B1)
Shield: 5 (PoryGone_SA2B)
There was a problem hiding this comment.
I would think it's probably slightly preferable to just list them each individually and match stylistically, I can also see some weird edge cases around item names with numbers in them if you try to add a quantity
There was a problem hiding this comment.
those would still be separated by the colon.
Rupees (500): 2 (Player3)
There was a problem hiding this comment.
Can item names contain colons (and do any currently)?
There was a problem hiding this comment.
just checked and quite a lot do. whether that makes the information difficult to parse is a different question. most notable ones would be stardew and raft which have items like Resource Pack: 2000 Money or Resource Pack: 5 Plastic
BaseClasses.py
Outdated
| else item.name | ||
| for item in chain.from_iterable(self.multiworld.precollected_items.values())] | ||
| if precollected_items: | ||
| outfile.write("\n\nPrecollected Items:\n\n") |
There was a problem hiding this comment.
I wonder whether this should just say "starting inventory," I'm not sure if there is value in distinguishing by this labeling between "the content of the start_inventory option" and "your actual starting inventory as it appears in precollected_items." But I'm not very strongly opinionated either way so I will leave this for others to give their thoughts
|
I've prompted the |
PoryGone
added
waiting-on: core-review
|
probably best to keep it similar to the starting inventory yaml option, to not confused the reader with additional terms |
Changed to "Starting Items", open to further thoughts. |
What is this fixing or adding?
Previously, items added to a player via
push_precollectedin a World implementation would not appear anywhere in the Spoiler Log, unless they were playthrough-relevant. This adds a section (when relevant) which displays all precollected items for the MultiWorld, whether added through World code or a player'sstart_inventory.I discovered this shortcoming amidst development of a forthcoming game implementation, wherein under some circumstances, items which are important to gameplay, but irrelevant to logic, can get added to the player's
start_inventory. I think it will be important for the players to be able to see that information in the spoiler log.This should behave similarly to the
Locations:section, in that it will list the player name for the item if there are multiple slots, but only the item name if only one slot is present. If multiple of the same item are added (by either mechanism), then each instance will be listed separately.How was this tested?
Several generations with a couple of supported worlds and a specially-modified SA2B world which adds a few filler items via
push_precollected, as well as YAMLs with items in thestart_inventory. Multi-slot and Single-slot generations were done, as well as generations without the above, to verify that if no precollected items exist, the section is not created.World for testing (rename as
.apworld, GitHub wouldn't let me upload with that extension):sa2b.zip
YAMLs for testing:
precollected_yamls.zip
If this makes graphical changes, please attach screenshots.
Sample spoiler output: