Archipelago icon indicating copy to clipboard operation
Archipelago copied to clipboard

Docs: explicitly document why get_filler_item_name may return non-IC.filler items, despite its name

Open Ixrec opened this issue 4 months ago • 3 comments

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

Ixrec avatar Dec 15 '25 14:12 Ixrec

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?)

duckboycool avatar Dec 17 '25 04:12 duckboycool

I feel like we should explicitly state that it's also normal and expected for worlds to return Traps here.

silasary avatar Dec 18 '25 03:12 silasary

Yeah, I thought about adding some (s) but felt it might just make things more confusing. Plus, my understanding is that in practice just returning a single hardcoded item name is fine for 99% of worlds. I'm not even sure if there is a scenario where that naive implementation could become noticeable or detrimental to a player.

Ixrec avatar Dec 18 '25 16:12 Ixrec

The mypy failure was "fatal: unable to access 'https://github.com/ArchipelagoMW/Archipelago/': SSL connection timeout". I need to update the branch anyway so hopefully that's a one-off.

Ixrec avatar Dec 23 '25 04:12 Ixrec