FelicitusNeko / meritous-ap

Lancer-X's procedurally-generated dungeon crawler, modified for Archipelago Multiworld
https://archipelago.gg/tutorial/Meritous/setup/en
GNU General Public License v3.0
0 stars 2 forks source link

support custom room counts from yaml #24

Open Rnd-Guy opened 2 months ago

Rnd-Guy commented 2 months ago

Updated 07/10/24

This top post will be kept up to date and should always provide a summary of the current state of the PR

Adds custom room count as a yaml option, as well as agate knife percentage

In order to do this, the following were done:

Testplays (of latest changes):

373988851-a65e43a6-87bb-45c9-bf9a-b94e028418cd Note that with these testplays, no crystal efficiency was picked up and no bosses were killed until the very end, so even vanilla cannot get 18 caches with this strategy hence it's not a consideration in this PR.

Expected total gems from enemies after multiplier (calculated at gen time):

373988367-cd1f9ad1-bd3d-48f8-8b7c-d6f71ac86a73

Other notes:

Gif of finding agate knife at room 100, with small dungeon displayed in map: (Note gif has a bug where if you pick up the knife then quit then picking the knife again does nothing but this has since been fixed) meritous-100-rooms

Changelog / Full test history

Rnd-Guy commented 2 months ago

In case it's helpful, I'll attach the apworld changes I've used to get this to work, feel free to change them as desired

__init__.py

@@ -57,6 +57,8 @@ class MeritousWorld(World):
        self.include_psi_keys = False
        self.item_cache_cost = 0
        self.death_link = False
+       self.rooms = 3000
+       self.agate_knife_percent = 100

    @staticmethod
    def _is_progression(name):

@@ -101,6 +103,8 @@ class MeritousWorld(World):
        self.include_psi_keys = self.options.include_psi_keys.value
        self.item_cache_cost = self.options.item_cache_cost.value
        self.death_link = self.options.death_link.value
+       self.rooms = self.options.rooms.value
+       self.agate_knife_percent = self.options.agate_knife_percent.value

    def create_regions(self):
        create_regions(self.multiworld, self.player)

@@ -177,5 +181,7 @@ class MeritousWorld(World):
        return {
            "goal": self.goal,
            "cost_scale": cost_scales[self.item_cache_cost],
-           "death_link": self.death_link
+           "death_link": self.death_link,
+           "rooms": self.rooms,
            "agate_knife_percent": self.agate_knife_percent,
        }

Options.py

from dataclasses import dataclass

import typing
-from Options import Option, DeathLink, Toggle, DefaultOnToggle, Choice, PerGameCommonOptions
+from Options import Option, DeathLink, Toggle, DefaultOnToggle, Choice, PerGameCommonOptions, Range, NamedRange

cost_scales = {

@@ -53,6 +53,37 @@ class ItemCacheCost(Choice):
    default = 0

+class Rooms(NamedRange):
+   """
+       Sets the total room count for the dungeon generated, which changes the length of the game.
+       Crystal gain will be multiplied to compensate for the reduced room count.
+
+       - **Original (default):** 3000 rooms, ie base game amount
+       - **Small:** 300 rooms
+       - **Medium:** 750 rooms
+       - **Large:** 1500 rooms
+   """
+   display_name = "Rooms"
+   range_start = 15
+   range_end = 3000
+   default = 3000
+   special_range_names = {
+       "original": 3000,
+       "small": 300,
+       "medium": 750,
+       "large": 1500,
+   }
+
+class AgateKnifePercent(Range):
+   """
+       How much % of rooms to discover to count as 100% room completion.
+       Once this % is reached, the agate knife will spawn
+   """
+   display_name = "Agate Knife %"
+   range_start = 0
+   range_end = 100
+   default = 100
+
@dataclass
class MeritousOptions(PerGameCommonOptions):
    goal: Goal

@@ -60,3 +91,5 @@ class MeritousOptions(PerGameCommonOptions):
    include_evolution_traps: IncludeEvolutionTraps
    item_cache_cost: ItemCacheCost
    death_link: DeathLink
+   rooms: Rooms
+   agate_knife_percent: AgateKnifePercent
Rnd-Guy commented 2 months ago

Found a bug with the branch, same as the bug shown in the gif of this PR:

meritous-no-cursed-seal

Rnd-Guy commented 2 months ago

Can indeed confirm this bug is indeed related to the order of operations for game start, possibly because by waiting for the connection we trigger the on_connect handlers too early.

The specific difference happens when we pick up the special item. We correctly identify that we need to pick it up, but we fail the if condition shown below whereas in normal branch we correctly go inside and receive our cursed seal.

void InternalCollectAPItem(uint64_t location)
{
  if (apStores[location / 24]->HasItemStored(location % 24))
    ReceiveItem((t_itemTypes)(apStores[location / 24]->GetStoredItem(location % 24) % AP_OFFSET),
                storeNames[location / 24]);
  else {
    std::list<int64_t> check;
    check.push_back(location + AP_OFFSET);
    ap->LocationScouts(check);
    ap->LocationChecks(check);
  }
}

Through trial and error it looks like it's specifically the case for when the connection is successful before we call ReadStoreData(), which makes sense given it's store related. However a lot of the load game functions cannot be reordered because map data needs to be read in a specific order. That is, ReadStoreData() must be called after RandomGenerateMap(), but connection must happen before RandomGenerateMap() and yet we need connection handlers to run after ReadStoreData().

This bug is really punching above it's weight class here D:

Rnd-Guy commented 2 months ago

My theory is that ReadStoreData() was resetting the store state, which is the second time it gets reset during initialisation (the first is during InitStores() which is usually after generate map but is now done at the start).

Normally this is harmless, as it gets reset twice before anything is added, but I think because we now finish connecting and handle connection handlers between the first and second store resets, funny business occurs. The bug no longer happens when I remove the second reset (ie remove CreateAPStores() below)

void ReadAPState()
{
  CreateAPStores();

  int progress = 0;
  nextCheckToGet = FRInt();
  for (auto store: apStores) {
    store->SetCostFactor(FRInt());
    for (size_t x = 0; x < store->GetLength(); x++) {
      auto collected = FRChar();
      if (collected) store->MarkCollected(x);
      auto hasItem = (bool)FRChar();
      if (hasItem) store->StoreItem(x, FRInt64());
    }
    UpdateLoadingScreen((float)(++progress + 1) / (IS_MAX + 1));
  }

It makes sense because

However the theory is broken because that should mean I can replicate it by starting a new game on a slot that has picked up a a special item, save game, quit and load game (to trigger the bug) and then try picking up the item again but it's working fine. Thankfully this is intended behaviour so it's not too bad of a thing if I can no longer replicate it but leaves uncertainty.

For now I think I'll just remove the call to CreateAPStores() and just monitor if this is an issue during further testplay.

Rnd-Guy commented 2 months ago

Just realised crystal counts shouldn't be scaled by room count but instead by monster count. Potentially can scale it by 10000 / monster count, as base game usually creates just over 10000 monsters, though I've seen sub 10k before.

For example, in the first gif above we have 100 rooms and 771 monsters. 30x less rooms gives me a crystal multiplier of 30 despite only having ~13x less monsters, off by more than a factor of two, which helps explain a bit of the surplus crystals that I'm noticing

Rnd-Guy commented 1 month ago

Did some more testing but unfortunately looks like there's an issue with crystal counts 🙁

Here's a chart of my crystals on a few runs with this PR, compared to a 3000 room count run I did on the sep 2024 big async. To ensure my comparison runs are consistent:

image

The three run sizes here (300, 750, 1500) were sorta arbitrary room counts that I chose to represent small, medium and large choices to be used as yaml options. Not much thought was particularly done to pick those numbers, but playtime wise these took about 40min, 1hr10 and 2 hours respectively to 100% which I think is really nice! It was suggested that possibly as a first approach, we stick to a limited set of room sizes to make the first implementation easier.

As for other testing results, I've not encountered any other major issues. I've confirmed the "not picking up agate/cursed seal" issue doesn't happen anymore after removing the CreateAPStores() call for example.

Once I figure out the crystal count issues, I think I'll be happy with how it is, but for now I'll put this to draft to reflect how it's not ready to be merged in this current state.

Edit: Found a bug where the crystals we drop on death (we lose 1/3, then 95% of that is turned into crystals we can pick back up) also get multiplied so we gain back more than we lost. Whoops! That might explain why 300 rooms gained so many more crystals.

Rnd-Guy commented 1 month ago

Made some changes to the crystal scaling algorithm to try to match vanilla a bit more, as well as a couple failsafes.

Summary The two main factors that currently govern how many crystals smaller dungeons get are:

Overshoot distance - aim to match room and thus enemy proportions Vanilla calculates stuff based on room distance from center, with a cap of 50. I've made the assumption that if a dungeon has a distance higher than 50, then it likely has more rooms considered "far away" enough to spawn harder enemies, which in turn affects gems dropped. Therefore by letting smaller dungeons also overshoot a little, I can mimic this behaviour to keep enemy proportions similar to vanilla.

I genned 50 vanilla dungeons and got an average max distance of 51.4, so I've multiplied the max distance of smaller dungeons by 50/51.4 (~97.2%) to mimic this behaviour, to keep enemy proportions roughly the same as what vanilla might have.

Min gem check - at least the minimum that vanilla can generate with I calculate the average number of gems dropped (for each monster add (min+max)/2) and then multiply that by the expected monster count. If this is below what vanilla can generate (~250000) then I redo the dungeon generation. This then prevents the smaller dungeons from being too volatile and not genning with enough crystals.

Since this skews the average, I've kept the expected monster count to be 10000, when the average is more like 10480 for vanilla, to keep the smaller dungeons from being too volatile and genning with too many crystals

Before: image

After: image

After with testplays: image

This is significantly better than the first attempt! The 750 and 1500 are still a tiny bit lower than I expected them to be but perhaps small sample size issue. I'm much more happier with this now.

Note these runs were done with the worst crystal strats (no efficiency, no bosses until the very end) and so even vanilla doesn't get 18 caches. As a result we only need to aim to be close enough to vanilla.

With this, I'm reopening this PR!

Rnd-Guy commented 3 weeks ago

I've made the changes according to your review but right now I can't actually compile it to see if it works D: I completely scuffed my c dev environment trying to figure out a good way to debug the game ^^;

I'll try to fix it this weekend

FelicitusNeko commented 3 weeks ago

If nothing else, it builds in Linux. I can try to finagle building for Windows, then give this a run in one or the other.

Rnd-Guy commented 3 weeks ago

Got my environment working I think, might still be scuffed though

However it looks like there's a bug when generating a dungeon that needs to regenerate due to my new crystal safety check. It looks like it's not cleaning up the enemies sufficiently and it's causing enemies to appear outside walls and cause horde rooms to be impossible, as well as glitching up the palette

image

To reproduce (I think), spam pressing new game until the loading bar noticeably resets (this means it's recreating the dungeon as it's not meeting some criteria which for small dungeons should only be my new min crystal check)

I've never seen this during all of my testing, so I dunno if this is due to my current scuffed environment or not but I'll do some more testing tomorrow

Rnd-Guy commented 3 weeks ago

Thankfully appears to be a simple fix, just needed to clean up enemies after each generate attempt

Successful 300 room run with latest version which reset gen a few times before starting, got 2 of the 17th caches which matches with my previous testing results when having 0 evo traps + no crystal efficiency image