search

ArchipelagoMW / Archipelago

Archipelago Multi-Game Randomizer and Server
https://archipelago.gg
Other
480 stars 638 forks source link

Core, some worlds: Rename sweep_for_events to sweep_for_advancements #3571

Closed NewSoupVi closed 1 month ago

NewSoupVi commented 3 months ago

Seems like we're going with the "advancement" naming now, and I have recent evidence that people think this function is referring to address = None. So let's try to get away from the old "event" naming and be consistent about "event" meaning address = None and "advancement" meaning "will be picked up by sweep_for_events / etc."

I wasn't sure about the event parameter on collect. I feel like that one might be used slightly differently? Lmk what you think about that one.

NewSoupVi commented 3 months ago

@Alchav (Can't request review)

beauxq commented 3 months ago

no deprecated sweep_for_events to not break apworlds?

NewSoupVi commented 3 months ago

no deprecated sweep_for_events to not break apworlds?

My bad, I had that, and then I accidentally removed it again lol

NewSoupVi commented 3 months ago

Approval from Espeon is implicit as per https://discord.com/channels/731205301247803413/1214608557077700720/1253206955879694336

Alchav commented 3 months ago

@Alchav (Can't request review)

What, do you really need my approval to rename a core function just because I use it?

NewSoupVi commented 3 months ago

Alchav (Can't request review)

What, do you really need my approval to rename a core function just because I use it?

That is the current procedure, yes - If something touches a world, the world maintainer has to approve.

We have discussed changing this in the (recent) past, some ppl argued that we shouldn't need to do this, but some world maintainers have said they want to continue to be required to sign off on anything touching their world.

Happy to discuss solutions in the Discord.

NewSoupVi commented 1 month ago

Oh thanks for the reapproval Alchav, did I accidentally ping you again? If so, I didn't mean to :D

NewSoupVi commented 1 month ago

Oh Violet, extremely smart and beautiful world maintainer of the highly acclaimed The Witness Randomizer for AP, are you willing to acquiesce to this most recent change?

NewSoupVi commented 1 month ago

Yes, esteemed and prolific core maintainer NewSoupVi, this change is beautiful and you should be proud of yourself, hats off to you my friend

NewSoupVi commented 1 month ago

Thank you lovely Violet, please continue on with your day, may it be an awesome one like you deserve!