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.

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: move option results to the World class instead of MultiWorld #993

Merged
merged 120 commits into from
Oct 10, 2023

Conversation

alwaysintreble
Copy link
Collaborator

What is this fixing or adding?

option access go brrr

How was this tested?

converted ror2 to using this system and generated/played seeds with it

If this makes graphical changes, please attach screenshots.

@alwaysintreble
Copy link
Collaborator Author

oh i forgot to make special case handling for lttp it appears. Don't really want to do that so unsure if I'll end up doing it to get this in earlier or if this gets put off until after more lttp refactoring 😓

Copy link
Member

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

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

See below. Also please update docs/world api.md

worlds/AutoWorld.py Outdated Show resolved Hide resolved
BaseClasses.py Outdated Show resolved Hide resolved
BaseClasses.py Outdated Show resolved Hide resolved
@black-sliver
Copy link
Member

I may have found another issue: test/general/__init__.py calls set_default_common_options - so we may have to modify that as well to provide self.options for that case.

@black-sliver black-sliver added is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization. labels Sep 5, 2022
@alwaysintreble
Copy link
Collaborator Author

I may have found another issue: test/general/__init__.py calls set_default_common_options - so we may have to modify that as well to provide self.options for that case.

Correct me if I'm wrong here, but it looks like the changes made to set_options covers this as that gets called first? And then we should just be able to remove set_default_common_options with the removal of setattr?

BaseClasses.py Outdated Show resolved Hide resolved
worlds/AutoWorld.py Outdated Show resolved Hide resolved
@black-sliver
Copy link
Member

I have not checked this, but I think if a world writes to self.world.option_name[player], that changes spoiler output?
writes to self.options[name] are not synchronized back

@black-sliver black-sliver mentioned this pull request Oct 16, 2022
@alwaysintreble
Copy link
Collaborator Author

I have not checked this, but I think if a world writes to self.world.option_name[player], that changes spoiler output? writes to self.options[name] are not synchronized back

This is correct actually. To support back compatibility until we rip out attr, the world options could be updated again right before output using the attributes, but this won't work with worlds that actually move over to this new system, so the only way to do this cleanly is to completely rip out the old system imo and I was kind of hoping not to do that quite yet as that is quite a bit more work and can't be easily refactored.

docs/world api.md Outdated Show resolved Hide resolved
docs/world api.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@kindasneaki kindasneaki left a comment

Choose a reason for hiding this comment

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

from .Options import DLCQuest_options, DLCQuestOptions, fetch_options
both DLCQuest_options and fetch_options dont exist anymore and shouldn't be imported

# in order for our game client to handle the generated seed correctly we need to know what the user selected
# for their difficulty and final boss HP
# a dictionary returned from this method gets set as the slot_data and will be sent to the client after connecting
# the options dataclass has a method to return a `Dict[str, Any]` of each option name provided and the option's value
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kind of afraid that this might make new world developers just stuff everything into slot data without thinking. Maybe mention that this should only done with what is actually needed by the client.

Options.py Outdated Show resolved Hide resolved
docs/world api.md Outdated Show resolved Hide resolved
@black-sliver black-sliver merged commit 7193182 into ArchipelagoMW:main Oct 10, 2023
12 checks passed
@alwaysintreble alwaysintreble deleted the options_dict branch October 12, 2023 11:23
DeamonHunter added a commit to DeamonHunter/Archipelago that referenced this pull request Oct 19, 2023
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
### New features

- ***Architect mode***
  Usually the cave is randomized by the game, meaning that each attempt will produce a different dungeon. However, with this new feature the player can, between runs, opt into keeping the same cave. If activated, they will then encounter the same floor layouts, same enemy spawns, and same red chest contents as on their previous attempt.   

- ***Custom item pool***
  Previously, the multiworld item pool consisted entirely of random blue chest items because, well, the permanent checks are blue chests and that's what one would normally get from these. While blue chest items often greatly increase your odds against regular enemies, being able to defeat the Master can be contingent on having an appropriate equipment setup of red chest items (such as Dekar blade) or even enemy drops (such as Hidora rock), most of which cannot normally be obtained from blue chests.
  With the custom item pool option, players now have the freedom to place any cave item into the multiworld itempool for their world.

- ***Enemy floor number, enemy sprite, and enemy movement pattern randomization***
  Experienced players can deduce a lot of information about the opposition they will be facing, for example: Given the current floor number, one can know in advance which of the enemy types will have a chance to spawn on that floor. And when seeing a particular enemy sprite, one can already know which enemy types one might have to face in battle if one were to come in contact with it, and also how that enemy group will move through the dungeon.
  Three new randomization options are added for players who want to spice up their game: one can shuffle which enemy types appear on which floor, one can shuffle which sprite is used by which enemy type, and one can shuffle which movement pattern is used by which sprite.

- ***EXP modifier***
  Just a simple multiplier option to allow people to level up faster. (For technical reasons, the maximum amount of EXP that can be awarded for a single enemy is limited to 65535, but even with the maximum allowed modifier of 500% there are only 6 enemy types in the cave that can reach this cap.)


### Balance change

- ***proportionally adjust chest type distribution to accommodate increased blue chest chance***
  One of the main problems that became apparent in the current version has to do with the distribution of chest contents. The game considers 6 categories, namely: consumable (mostly non-restorative), consumable (restorative), blue chest item, spell, gear, and weapon. Since only blue chests count as multiworld locations, we want to have a mechanism to customize the blue chest chance.
  Given how the chest types are detetermined in game, a naive implementation of an increased blue chest chance causes only the consumable chance to be decreased in return. In practice, this has resulted in some players of worlds with a high blue chest chance struggling (more than usual) to keep their party alive because they were always low on comsumables that restore HP and MP.
  The new algorithm tries to avoid this one-sided effect by having an increase in blue chest chance resulting in a decrease of all other types, calculated in such a way that the relative distribution of the other 5 categories stays (approximately) the same.


### Bug fixes

- ***prevent using party member items if character is already in party***
  This should have been changed at the same time that 6eb0062 was made, but oh well... 

- ***fix glitched sprite when opening a chest immediately after receiving an item***
  When opening a chest right after receiving a multiworld item (such that there were two item get animations in the exact same iteration of the game main loop), the item from the chest would display an incorrect sprite in the wrong place. Fixed by cleaning up some relevant memory addresses after getting the multiworld item.

- ***fix death link***
  There was a condition in `deathlink_kill_player` that looked kinda smart (it checked the time against `last_death_link`), but actually wasn't smart at all because `deathlink_kill_player` is executed as an async task and the main thread will update `last_death_link` after creating the task, meaning that whether or not the incoming death link would actually be passed to the game seems to have been up to a race condition. Fixed by simply removing that check.


### Other

- ***add Lufia II Ancient Cave (and SMW) to the network diagram***
  These two games were missing from the SNES sector.

- ***implement get_filler_item_name***
  Place a restorative consumable instead of a completely random item. (Now the only known problem with item links in lufia2ac is... that noone has ever tested item links. But this should be an improvement at least. Anyway, now ArchipelagoMW#1172 can come ;)
  And btw., if you think that the implementation of random selection in this method looks weird, that's because it is indeed weird. (It tries to recreate the algorithm that the game itself uses when it generates a replacement item for a chest that would contain a spell that the party already knows.)

- ***store all options in a dataclass***
  This is basically like using ArchipelagoMW#993 (but without actual support from core). It makes the lufia2ac world code much nicer to maintain because one doesn't have to change 5 different places anymore when adding or renaming an option.

- ***remove master_hp.scale***
  I have to admit: `scale` was a mistake. Never have I seen a single option value cause so many user misconceptions. Some people assume it affects enemies other than the Master; some people assume it affects stats other than HP; and many people will just assume it is a magic option that will somehow counterbalance whatever settings combination they are currently trying to shoot themselves in the foot with.
  On top of that, the `scale` mechanism probably doesn't provide a good user experience even when used for its intended purpose (since having reached floor XY in general doesn't mean you will have the power to deplete XY% of the Masters usual HP; especially given that, due to the randomness of loot, you are never guaranteed to be able to defeat the vanilla Master even when you have cleared 100% of the floors).
  The intended target audience of the `master_hp` option are people who want to fight the Master (and know how to fight it), but also want to lessen (to a degree of their choosing) the harsh dependence on the specific equipment setups that are usually required to win this fight even when having done all 99 floors. They can achieve this by setting the `master_hp` option to a numeric value appropriate for the level of challenge they are seeking. Therefore, nothing of value should be lost by removing the special `scale` value from the `master_hp` option, while at the same time a major source of user confusion will be eliminated.

- ***typing***
  This (combined with the switch to the option dataclass) greatly reduces the typing problems in the lufia2ac world. The remaining typing errors mostly fall into 4 categories:
  1. Lambdas with defaults (which seem to be incorrectly reported as an error due to a mypy bug)
  1. Classmethods that return instances (which could probably be improved using PEP 673 "Self" types, but that would require Python 3.11 as the minimum supported version)
  1. Everything that inherits from TextChoice (which is a typing mess in core)
  1. Everything related to asar.py (which does not have proper typing and lies outside of this project)

## How was this tested?

https://discord.com/channels/731205301247803413/1080852357442707476 and others
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
…rchipelagoMW#993)

🤞 

* map option objects to a `World.options` dict

* convert RoR2 to options dict system for testing

* add temp behavior for lttp with notes

* copy/paste bad

* convert `set_default_common_options` to a namespace property

* reorganize test call order

* have fill_restrictive use the new options system

* update world api

* update soe tests

* fix world api

* core: auto initialize a dataclass on the World class with the option results

* core: auto initialize a dataclass on the World class with the option results: small tying improvement

* add `as_dict` method to the options dataclass

* fix namespace issues with tests

* have current option updates use `.value` instead of changing the option

* update ror2 to use the new options system again

* revert the junk pool dict since it's cased differently

* fix begin_with_loop typo

* write new and old options to spoiler

* change factorio option behavior back

* fix comparisons

* move common and per_game_common options to new system

* core: automatically create missing options_dataclass from legacy option_definitions

* remove spoiler special casing and add back the Factorio option changing but in new system

* give ArchipIDLE the default options_dataclass so its options get generated and spoilered properly

* reimplement `inspect.get_annotations`

* move option info generation for webhost to new system

* need to include Common and PerGame common since __annotations__ doesn't include super

* use get_type_hints for the options dictionary

* typing.get_type_hints returns the bases too.

* forgot to sweep through generate

* sweep through all the tests

* swap to a metaclass property

* move remaining usages from get_type_hints to metaclass property

* move remaining usages from __annotations__ to metaclass property

* move remaining usages from legacy dictionaries to metaclass property

* remove legacy dictionaries

* cache the metaclass property

* clarify inheritance in world api

* move the messenger to new options system

* add an assert for my dumb

* update the doc

* rename o to options

* missed a spot

* update new messenger options

* comment spacing

Co-authored-by: Doug Hoskisson <[email protected]>

* fix tests

* fix missing import

* make the documentation definition more accurate

* use options system for loc creation

* type cast MessengerWorld

* fix typo and use quotes for cast

* LTTP: set random seed in tests

* ArchipIdle: remove change here as it's default on AutoWorld

* Stardew: Need to set state because `set_default_common_options` used to

* The Messenger: update shop rando and helpers to new system; optimize imports

* Add a kwarg to `as_dict` to do the casing for you

* RoR2: use new kwarg for less code

* RoR2: revert some accidental reverts

* The Messenger: remove an unnecessary variable

* remove TypeVar that isn't used

* CommonOptions not abstract

* Docs: fix mistake in options api.md

Co-authored-by: Doug Hoskisson <[email protected]>

* create options for item link worlds

* revert accidental doc removals

* Item Links: set default options on group

* change Zillion to new options dataclass

* remove unused parameter to function

* use TypeGuard for Literal narrowing

* move dlc quest to new api

* move overcooked 2 to new api

* fixed some missed code in oc2

* - Tried to be compliant with 993 (WIP?)

* - I think it all works now

* - Removed last trace of me touching core

* typo

* It now passes all tests!

* Improve options, fix all issues I hope

* - Fixed init options

* dlcquest: fix bad imports

* missed a file

* - Reduce code duplication

* add as_dict documentation

* - Use .items(), get option name more directly, fix slot data content

* - Remove generic options from the slot data

* improve slot data documentation

* remove `CommonOptions.get_value` (ArchipelagoMW#21)

* better slot data description

Co-authored-by: black-sliver <[email protected]>

---------

Co-authored-by: el-u <[email protected]>
Co-authored-by: Doug Hoskisson <[email protected]>
Co-authored-by: Doug Hoskisson <[email protected]>
Co-authored-by: black-sliver <[email protected]>
Co-authored-by: Alex Gilbert <[email protected]>
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
…rchipelagoMW#993)

🤞 

* map option objects to a `World.options` dict

* convert RoR2 to options dict system for testing

* add temp behavior for lttp with notes

* copy/paste bad

* convert `set_default_common_options` to a namespace property

* reorganize test call order

* have fill_restrictive use the new options system

* update world api

* update soe tests

* fix world api

* core: auto initialize a dataclass on the World class with the option results

* core: auto initialize a dataclass on the World class with the option results: small tying improvement

* add `as_dict` method to the options dataclass

* fix namespace issues with tests

* have current option updates use `.value` instead of changing the option

* update ror2 to use the new options system again

* revert the junk pool dict since it's cased differently

* fix begin_with_loop typo

* write new and old options to spoiler

* change factorio option behavior back

* fix comparisons

* move common and per_game_common options to new system

* core: automatically create missing options_dataclass from legacy option_definitions

* remove spoiler special casing and add back the Factorio option changing but in new system

* give ArchipIDLE the default options_dataclass so its options get generated and spoilered properly

* reimplement `inspect.get_annotations`

* move option info generation for webhost to new system

* need to include Common and PerGame common since __annotations__ doesn't include super

* use get_type_hints for the options dictionary

* typing.get_type_hints returns the bases too.

* forgot to sweep through generate

* sweep through all the tests

* swap to a metaclass property

* move remaining usages from get_type_hints to metaclass property

* move remaining usages from __annotations__ to metaclass property

* move remaining usages from legacy dictionaries to metaclass property

* remove legacy dictionaries

* cache the metaclass property

* clarify inheritance in world api

* move the messenger to new options system

* add an assert for my dumb

* update the doc

* rename o to options

* missed a spot

* update new messenger options

* comment spacing

Co-authored-by: Doug Hoskisson <[email protected]>

* fix tests

* fix missing import

* make the documentation definition more accurate

* use options system for loc creation

* type cast MessengerWorld

* fix typo and use quotes for cast

* LTTP: set random seed in tests

* ArchipIdle: remove change here as it's default on AutoWorld

* Stardew: Need to set state because `set_default_common_options` used to

* The Messenger: update shop rando and helpers to new system; optimize imports

* Add a kwarg to `as_dict` to do the casing for you

* RoR2: use new kwarg for less code

* RoR2: revert some accidental reverts

* The Messenger: remove an unnecessary variable

* remove TypeVar that isn't used

* CommonOptions not abstract

* Docs: fix mistake in options api.md

Co-authored-by: Doug Hoskisson <[email protected]>

* create options for item link worlds

* revert accidental doc removals

* Item Links: set default options on group

* change Zillion to new options dataclass

* remove unused parameter to function

* use TypeGuard for Literal narrowing

* move dlc quest to new api

* move overcooked 2 to new api

* fixed some missed code in oc2

* - Tried to be compliant with 993 (WIP?)

* - I think it all works now

* - Removed last trace of me touching core

* typo

* It now passes all tests!

* Improve options, fix all issues I hope

* - Fixed init options

* dlcquest: fix bad imports

* missed a file

* - Reduce code duplication

* add as_dict documentation

* - Use .items(), get option name more directly, fix slot data content

* - Remove generic options from the slot data

* improve slot data documentation

* remove `CommonOptions.get_value` (ArchipelagoMW#21)

* better slot data description

Co-authored-by: black-sliver <[email protected]>

---------

Co-authored-by: el-u <[email protected]>
Co-authored-by: Doug Hoskisson <[email protected]>
Co-authored-by: Doug Hoskisson <[email protected]>
Co-authored-by: black-sliver <[email protected]>
Co-authored-by: Alex Gilbert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.