Skip to content

Conversation

@Ixrec
Copy link
Collaborator

@Ixrec Ixrec commented Dec 15, 2025

What is this fixing or adding?

Clearing up some longstanding confusion around get_filler_item_name not always returning (the name(s) of) filler item(s). I don't have a better idea for the method's name, and my understanding is that renaming methods is very difficult in this project anyway, but we can make the docstring more explicit about the method's real contract.

This proposed text reflects my understanding of this method based on past conversations such as https://discord.com/channels/731205301247803413/1214608557077700720/1365116330625204295 where veteran devs explicitly draw a distinction between "filler" the ItemClassification and "filler" that get_filler_item_name() is trying to generate.

I thought of doing this today because starting at https://discord.com/channels/731205301247803413/1214608557077700720/1450025033266233377 there was a semi-heated discussion of this same familiar confusion, which appeared to include a number of incorrect assertions that get_filler_item_name does always return filler (though even its default implementation doesn't).

How was this tested?

reading

If this makes graphical changes, please attach screenshots.

N/A

@github-actions github-actions bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Dec 15, 2025
@duckboycool
Copy link
Contributor

A couple comments:

  • I think The returned item name will be passed to create_item() could be slightly misleading, since it might imply that it's the only way the function could be used. Maybe just mentioning that create_filler specifically will do that is better.
  • I feel like the wording here is unnecessarily strict about giving non-filler item names. It could make sense to put items of other classifications there even if your world does have filler items, and moreover there's often filler-class items that actually aren't appropriate for this function. I think it should describe that it needs to give an item that makes sense to have an arbitrary amount of regardless of class more explicitly. (Also there's that whole thing where apparently these are given filler class regardless of what create_item gives it originally, but that's probably not needed here?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants