HK: Removes Vanilla Items from ItemPool and Uses Grimmchild1 when relevant#2898
HK: Removes Vanilla Items from ItemPool and Uses Grimmchild1 when relevant#2898ThePhar merged 7 commits intoArchipelagoMW:mainfrom
Conversation
…g logicless items and making logic-relevant items as events)
…Grimmchild2 which is default
github-actions
bot
added
the
waiting-on: peer-review
|
Relevant issues on the HK side |
|
Most notably the idea for the event items was to make it an option (default off) to make them real locations still which I don't imagine happened here based off the pr description |
|
I had tried a similar change long ago, and kind of forgot about it, but I found that if shop locations didn't exist, the shops would be empty or bug out? Do shops work properly with this? |
what settings are you thinking about? |
also correct, did not see that request nor handle it, |
|
For shops there might actually be some client side handling needed for the vanilla items to appear in the shops, I think that might be the issue here. For example if you randomize nothing what does sly have. What about it you randomize charms and nothing else? |
|
confirmed this fix does not populate empty shop locations |
|
My solution was to just always create shop locations. I intended to eventually bring it up for discussion and do some test games but kind of just forgot about it. |
|
Itemchanger has default item enums for shop locations so I can just use those and they will be treated as vanilla. I am a little surprised that the location is created in IC at all when nothing is randomized but it can be dealt with |
Yes, I would still like this, I support the idea of the coop use case but hate that it negatively impacts probably 90+% of the player base by being the only option |
Done, again, lightly tested |
Also this change is dependent on this game mod update (or confirmation that some other solution is needed to fix shop items, like the previously mentioned always make shop items AP items) |
|
Code looks good now as far as I'm concerned but will modify and test the client changes before giving an actual approval |
|
HK client can accommodate this as of ArchipelagoMW-HollowKnight/Archipelago.HollowKnight@7232935 and will be present starting in client version 0.4.0 |
BadMagic100
added
the
waiting-on: world-maintainer
There was a problem hiding this comment.
During a longer playtest we found that with this change, hinting for "grimmchild" will suggest grimmchild2 by fuzzy matching. I'm not sure if this is an issue in this change, or with the webhost, or not an issue at all and it will just pick one because both grimmchild1 and grimmchild2 match closely. The actual root cause should be investigated further and addressed here if appropriate.
I'd also like to add an item group for grimmchild containing both of these
|
from my skimming through the fuzzy matching it looks like it finds both grimmchild1 and grimmchild2 and can't decide between the two so stops hinting, but with the item group it finds that instead and returns the relevant hint |
|
Oh, the only other thing maybe worth considering here is start inventory - custom skills (nail directions, focus, swim) still are sent via AP. No idea how the client will handle it they are removed (I think it should be ok but will need testing if changed) |
|
the in-world starting inventory items (like focus/swim/nails) currently bypass the 'vanilla' check because they fall under the |
|
Right, I'm wondering if that should be changed. It's not technically related to the hint point issue but it is in line with the server not being involved with unrandomized items. |
|
ah, if that's what you're worried about, I would say it's out of scope for this PR but also the information would need to get to the client in some way, either as-is in start inventory, in slot data, or trying to interpret based on the items not being in the item pool. |
|
Well, all of those items the client doesn't even need to know about because only the existence of the custom skill (or the corresponding setting to randomize it being on) will remove the skill. They don't exist in vanilla. As is, sending them is completely redundant; it loads the item, removing the ability to focus/swim/etc and then immediately grants you the ability it just took (or, on the off chance that isn't how it works, that is how it should work and I would consider it a client bug) I think if there is an easy (1-2 line) fix for it I would like it, otherwise we can leave it since RC is apparently approaching and I'd rather have it in with that quirk than not have it in at all |
|
Oh, but logic needs to know about them somehow, I guess that makes it kinda icky |
ThePhar
left a comment
ThePhar
left a comment
There was a problem hiding this comment.
Logic looks fine, mostly just name/doccomment that needs some tweaking imo.
…pelagoMW#2922) * remove component checking * fix missing deathlink messages * move reads under deathlink check Core: Fix OptionList and OptionSet to allow Iterable of Iterable (ArchipelagoMW#2911) * fix, maybe * typegard for iterable of any * wow I'm so tired I just changed the method name without changing what it actually does... * also exclude bytes in is_iterable_but_str * apply pr comments * Update Utils.py Co-authored-by: Doug Hoskisson <[email protected]> * Revert "also exclude bytes in is_iterable_but_str" This reverts commit cf087d2. --------- Co-authored-by: Doug Hoskisson <[email protected]> Docs: Added snes9x-nwa as recommended emulator to the setup guides for SNES games (ArchipelagoMW#1778) * Added snes9x-nwa as recommended emulator to the setup guides * Removed snes9x-nwa from the setup guides of DKC3 and SMW * Update worlds/alttp/docs/multiworld_en.md Co-authored-by: Aaron Wagener <[email protected]> * Removed duplicate text Minor grammar and spelling fixes * Unified required software for SM, SMZ3 and SoE with ALTTP * Added instructions for usage of BSNES-Plus for ALTTP, SM and SMZ3 --------- Co-authored-by: Aaron Wagener <[email protected]> KH2: Update all instances of multiworld.option_name to option.option_name (ArchipelagoMW#2634) * update the multiworld to options * Update worlds/kh2/Rules.py Co-authored-by: Exempt-Medic <[email protected]> * does this work * namine sketches * wrong branch :) --------- Co-authored-by: Exempt-Medic <[email protected]> The Messenger: fix items accessibility reachability bug due to new rules (ArchipelagoMW#2937) CI: Don't auto-remove content based labels (ArchipelagoMW#2941) Docs: improve AutoWorld method docstrings (ArchipelagoMW#2509) * clarify some autoworld docstrings * revert accidental change The Witness: Don't unnecessarily break people's 0.4.4 yamls (ArchipelagoMW#2940) kvui: allow sorting hints in the hint tab (ArchipelagoMW#2684) MultiServer: send new read_hints datastore values on change (ArchipelagoMW#2558) The Witness: Obelisk Keys (ArchipelagoMW#2805) Core: String comparison with FreeText class (ArchipelagoMW#2942) CommonClient: use rich text for /received (ArchipelagoMW#2715) CommonClient: Fix item link group name when member slot name contains brackets (ArchipelagoMW#2794) SMW: v2.0 Content Update (ArchipelagoMW#2762) Changelog: Features: - New optional Location Checks - 3-Up Moons - Hidden 1-Ups - Bonus Blocks - Blocksanity - All blocks that contain coins or items are included, with the exception of: - Blocks in Top Secret Area & Front Door/Bowser Castle - Blocks that are unreachable without glitches/unreasonable movement - New Items - Special Zone Clear - New Filler Items - 1 Coin - 5 Coins - 10 Coins - 50 Coins - New Trap Items - Reverse Trap - Thwimp Trap - SFX Shuffle - Palette Shuffle Overhaul - New Curated Palette can now be used for the Overworld and Level Palette Shuffle options - Foreground and Background Shuffle options have been merged into a single setting - Max possible Yoshi Egg value is 255 - UI in-game is updated to handle 3-digits - New `Display Received Item Popups` option: `progression_minus_yoshi_eggs` Quality of Life: - In-Game Indicators are now displayed on the map screen for location checks and received items - In-level sprites are displayed upon receiving certain items - The Camera Scroll unlocking is now only enabled on levels where it needs to be - SMW can now handle receiving more than 255 items - Significant World Code cleanup - New Options API - Removal of `world: MultiWorld` across the world - The PopTracker pack now has tabs for every level/sublevel, and can automatically swap tabs while playing if connected to the server Bug Fixes: - Several logic tweaks/fixes "Major credit to @TheLX5 for being the driving force for almost all of this update. We've been collaborating on design and polish of the features for the last few months, but all of the heavy lifting was all @TheLX5." Core: typing for `Option.default` and a few other ClassVars (ArchipelagoMW#2899) * Core: typing for `Option.default` and a few other `Option` class variables This is a replacement for ArchipelagoMW#2173 You can read discussion there for issues we found for why we can't have more specific typing on `default` instead of setting a default in `Option` (where we don't know the type), we check in the metaclass to make sure they have a default. * NumericOption doesn't need the type annotation that brings out the mypy bug * SoE default ClassVar Core: add list/dict merging feature to triggers (ArchipelagoMW#2793) * proof of concept * add dict support, block top/game level merge * prevent key error when option being merged is new * update triggers guide * Add documentation about add/remove/replace * move to trailing name instead of proper tag * update docs * confirm types * Update Utils.py * Update Generate.py * pep8 * move to + syntax * forgot to support sets * specify received type of type error * Update Generate.py Co-authored-by: Fabian Dill <[email protected]> * Apply suggestion from review * add test for update weights * move test to new test case * Apply suggestions from code review Co-authored-by: black-sliver <[email protected]> --------- Co-authored-by: Fabian Dill <[email protected]> Co-authored-by: black-sliver <[email protected]> CI: build: create setup (ArchipelagoMW#2936) * CI: build: create setup also add /DNO_SIGNTOOL to inno_setup.iss * CI: build: trigger when changing setup-related files Shivers: Renaming for clarity and consistency (ArchipelagoMW#2869) * Moves plaque location to front for better tracker referencing. * Tiki should be Shaman. * Hanging should be Gallows. * Merrick spelling. * Clarity change. FFMQ: Update Map Shuffle Seed description (ArchipelagoMW#2658) * Update Map Shuffle Seed description * Update worlds/ffmq/Options.py Co-authored-by: Exempt-Medic <[email protected]> --------- Co-authored-by: Exempt-Medic <[email protected]> Core: fix incorrect ordering on the always_allow static method (ArchipelagoMW#2938) CI: update actions (ArchipelagoMW#2943) HK: Removes Vanilla Items from ItemPool and Uses Grimmchild1 when relevant (ArchipelagoMW#2898) KDL3: Ensure all abilities accessible on non-minimal (ArchipelagoMW#2929) Pokemon Emerald: v2 Update (ArchipelagoMW#2918) Core: add layer for patches that don't use `Patch.py` (ArchipelagoMW#2889) * Core: add layer for patches that don't use `Patch.py` * bump container version * APAutoPatchInterface name * mystic quest change * OoT and Adventure changes * missed name in docstring * container version compatibility Lingo: Pre-compile datafile to improve loading time (ArchipelagoMW#2829) CommonClient: Don't retry connection when connection details are invalid (ArchipelagoMW#2831) Stardew Valley: 5.x.x - The Allsanity Update (ArchipelagoMW#2764) Major Content update for Stardew Valley, including the following features - Major performance improvements all across the Stardew Valley apworld, including a significant reduction in the test time - Randomized Farm Type - Bundles rework (Remixed Bundles and Missing Bundle!) - New Settings: * Shipsanity - Shipping individual items * Monstersanity - Slaying monsters * Cooksanity - Cooking individual recipes * Chefsanity - Learning individual recipes * Craftsanity - Crafting individual items - New Goals: * Protector of the Valley - Complete every monster slayer goal * Full Shipment - Ship every item * Craftmaster - Craft every item * Gourmet Chef - Cook every recipe * Legend - Earn 10 000 000g * Mystery of the Stardrops - Find every stardrop (Maguffin Hunt) * Allsanity - Complete every check in your slot - Building Shuffle: Cheaper options - Tool Shuffle: Cheaper options - Money rework - New traps - New isolated checks and items, including the farm cave, the movie theater, etc - Mod Support: SVE [Albrekka] - Mod Support: Distant Lands [Albrekka] - Mod Support: Hat Mouse Lacey [Albrekka] - Mod Support: Boarding House [Albrekka] Co-authored-by: Witchybun <[email protected]> Co-authored-by: Witchybun <[email protected]> Co-authored-by: Jouramie <[email protected]> Co-authored-by: Alchav <[email protected]> Launcher: make scrollbar more prominent (ArchipelagoMW#2955) TUNIC: Updated display name for a few options (ArchipelagoMW#2953) SMW: Add CHANGELOG.md (ArchipelagoMW#2947) Celeste 64: Add CHANGELOG.md (ArchipelagoMW#2948) DKC3: Add CHANGELOG.md (ArchipelagoMW#2946) Core: increment version (ArchipelagoMW#2958) SA2B: Add CHANGELOG.md (ArchipelagoMW#2945) The Witness: Add newly submitted junk hints (ArchipelagoMW#2949) OoT: Entrance Spoiler Fixes (ArchipelagoMW#2500) Stardew Valley: Added a Great Combat requirement to an entrance that could block its own key (ArchipelagoMW#2959) SC2: Multi-campaign (ArchipelagoMW#2954) Adds HotS, LotV and NCO campaigns to SC2 game. The world's name has changed to reflect that (it's not only Wings of Liberty now) The client was patched in a way that can still join to games generated prior this change --------- Co-authored-by: Magnemania <[email protected]> Co-authored-by: EnvyDragon <[email protected]> Co-authored-by: Matthew <[email protected]> Co-authored-by: hopop201 <[email protected]> Co-authored-by: Salzkorn <[email protected]> Co-authored-by: genderdruid <[email protected]> Co-authored-by: MadiMadsen <[email protected]> Co-authored-by: neocerber <[email protected]> Co-authored-by: Exempt-Medic <[email protected]> Co-authored-by: Fabian Dill <[email protected]> Zork Grand Inquisitor: Implement New Game (ArchipelagoMW#2539) Adds Archipelago support for Zork Grand Inquisitor, the 1997 point-and-click PC adventure game. The client (based on `CommonClient`), on top of its regular Archipelago duties, fully handles the randomization of the game and the monitoring / modification of the game state. No game modding needed at all; the player is ready to play an Archipelago seed if they can play the vanilla game through ScummVM. The "reverse engineering" (there's likely a better term for this...) of the game is my own original work and I included an MIT license at the root of my world directory. A PopTracker pack was also created to help people learn the game: https://github.com/SerpentAI/ZorkGrandInquisitorAPTracker TUNIC: Implement support for connection plando (ArchipelagoMW#2864) The Witness: Add junk hint for Zork: Grand Inquisitor (ArchipelagoMW#2961) SMW: Increment Required Client Version (ArchipelagoMW#2962) Core: implement APProcedurePatch and APTokenMixin (ArchipelagoMW#2536) * initial work on procedure patch * more flexibility load default procedure for version 5 patches add args for procedure add default extension for tokens and bsdiff allow specifying additional required extensions for generation * pushing current changes to go fix tloz bug * move tokens into a separate inheritable class * forgot the commit to remove token from ProcedurePatch * further cleaning from bad commit * start on docstrings * further work on docstrings and typing * improve docstrings * fix incorrect docstring * cleanup * clean defaults and docstring * define interface that has only the bare minimum required for `Patch.create_rom_file` * change to dictionary.get * remove unnecessary if statement * update to explicitly check for procedure, restore compatible version and manual override * Update Files.py * remove struct uses * ensure returning bytes, add token type checking * Apply suggestions from code review Co-authored-by: Doug Hoskisson <[email protected]> * pep8 --------- Co-authored-by: beauxq <[email protected]> Co-authored-by: Doug Hoskisson <[email protected]> Pokemon Emerald: Bump required client version (ArchipelagoMW#2963)
5 participants
What is this fixing or adding?
All logic-less items when not randomized will not be added to the itempool and should default in-game to their vanilla placements/rewards
All logic-relevant items when not randomized will also not be added to the itempool but will be represented as Events so logic can remain unchanged (for now)
Also adds the Grimmchild1 item to the itempool instead of Grimmchild2 when relevant (when both flames and charms are shuffled), otherwise uses Grimmchild2 as default
How was this tested?
quickly
tested logicless removal with geo rocks and soul totems off, and with lore on
confirmed the off locations were not present in the spoiler log, were not sent to the server by the game, and did default to vanilla items in-game
confirmed the on locations were present in the spoiler log, did send to the server by the game, and did grant randomized items when checked in-game
only confirmed the Grimmchild updates in spoiler log, but did test that:
with both charm + flames on, Grimmchild1 was shuffled into the multiworld
with only charm on, Grimmchild2 was shuffled into the multiworld
with only flames on, Grimmchild2 was set to its vanilla location (and supposedly an Event to be handled by the game per vanilla rules, but did not confirm)
If this makes graphical changes, please attach screenshots.