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

Update Maps::getBiomeType and Maps::getBiomeTypeWithRef #4128

Merged
merged 11 commits into from
Jan 21, 2024

Conversation

Bumber64
Copy link
Contributor

@Bumber64 Bumber64 commented Jan 2, 2024

I've updated the functions getBiomeType and getBiomeTypeWithRef based on reverse-engineering of v50.11.

The only real changes seem to be:
potential_tropical (rather than tropical) is sufficient for OCEAN_TROPICAL
region.vegetation isn't used at all, instead replaced by region.rainfall checks

getBiomeType and getBiomeTypeWithRef now do bounds checking on the region coords by calling getRegionBiome, then doing a CHECK_NULL_POINTER on the region. I'd prefer to return biome_type::NONE here instead of throwing an exception, but that doesn't exist so we'd have to add that to structures if appropriate.

I indented the DFHack and Maps namespaces in Maps.h, and inlined getBiomeType there.

* Indent namespaces
* Inline getBiomeType
* Update `getBiomeTypeWithRef` and related fns to v50.11
* Bounds checking on region coords via `getRegionBiome`
* Utility fn `check_tropicality` removed, handled directly in `getBiomeTypeWithRef`
* Utility fn `get_region_parameter` renamed to `basic_wet_dry_effect`
@Bumber64
Copy link
Contributor Author

Bumber64 commented Jan 2, 2024

Anybody know why getBiomeTypeWithRef exists in the first place? Embark assistant uses it to check a region biome using an adjacent tile's tropicality/latitude (with all other regional parameters remaining the same,) but I'm unsure of the reason for doing so.

@Bumber64 Bumber64 changed the title Update getBiomeType and getBiomeTypeWithRef Update Maps::getBiomeType and Maps::getBiomeTypeWithRef Jan 2, 2024
@ab9rf
Copy link
Member

ab9rf commented Jan 3, 2024

i'm trying to understand the motivation behind this. is the current implementation incorrect in some way?

for the history, see #1392 and #1394

@Bumber64
Copy link
Contributor Author

Bumber64 commented Jan 4, 2024

i'm trying to understand the motivation behind this. is the current implementation incorrect in some way?

For one, the old code was reporting a regional ocean tile as OCEAN_TEMPERATE while the new code says it's OCEAN_TROPICAL. (Swapping region.vegetation for region.rainfall checks may or may not have created changes. Haven't tested this since I ran into the ocean issue first, meaning there are changes regardless.)

I used a lua script iterating all region tiles to test, but found it too much a hassle to verify what the game UI says (even in a pocket world,) since there's no keyboard support for embark screen and I'd basically have to use trial and error to embark on the right tile to verify region coords. Just going to assume the current disassembled function (FUN_140bfe460 v50.11 win64 Steam) is correct on oceans, otherwise there's a Bay12 bug.

AFAIK, getBiomeTypeWithRef (where we use region_ref_y (world_ref_coord_y) for tropicality) doesn't exist in Bay12 code, so I'm just swapping the ref coord in where the old DFHack function does and have no way of verifying it without knowing the intent.

@myk002 myk002 requested a review from ab9rf January 5, 2024 08:09
@myk002
Copy link
Member

myk002 commented Jan 19, 2024

do you have a map where I can see the changes in interpretation? I tried to verify on the maps I have, but I'm not sure how to specifically get a biome with potential_tropical set.

@Bumber64
Copy link
Contributor Author

Bumber64 commented Jan 21, 2024

do you have a map where I can see the changes in interpretation? I tried to verify on the maps I have, but I'm not sure how to specifically get a biome with potential_tropical set.

Using this branch: https://github.com/Bumber64/dfhack/tree/test-biomes
Generate a Pocket Region with North pole and seed test.
Use this Lua script:

function test_biomes()
  local wd = df.global.world.world_data
  for x=0, wd.world_width-1 do
    for y=0, wd.world_height-1 do
      local b_new = df.biome_type[dfhack.maps.getBiomeType(x,y)]
      local b_old = df.biome_type[dfhack.maps.getBiomeTypeOld(x,y)]
      if b_new ~= b_old then
        print(("(%d, %d): %s -> %s"):format(x, y, b_old, b_new))
      end
    end
  end
  print("Done!")
end

Output should be:

(14, 11): OCEAN_TEMPERATE -> OCEAN_TROPICAL
(15, 11): OCEAN_TEMPERATE -> OCEAN_TROPICAL
(15, 12): OCEAN_TEMPERATE -> OCEAN_TROPICAL
(16, 11): OCEAN_TEMPERATE -> OCEAN_TROPICAL
(16, 12): OCEAN_TEMPERATE -> OCEAN_TROPICAL
Done!

These regions should be potential_tropical but not tropical.

Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

nice. thanks!

@myk002 myk002 merged commit edc27c5 into DFHack:develop Jan 21, 2024
14 checks passed
@Bumber64 Bumber64 deleted the patch-1 branch January 22, 2024 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants