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

Feature: Water tiles have a depth #7924

Draft
wants to merge 33 commits into
base: master
from
Draft

Conversation

@nielsmh
Copy link
Contributor

nielsmh commented Jan 11, 2020

Allocate 4 bits in M3 to store the depth of MP_WATER tiles.

In the base game, depth affects the cost of clearing (and otherwise modifying) water tiles, and the meaning of "river" versus "sea" speed for ships has changed to "shallow" versus "deep" water.
There is support for NewGRFs to supply a table of water surface sprites for different depth levels, via action 5. Maybe this should be added to the baseset.
Additionally, NewGRF industries will be able to check the water depth, so e.g. dredging sites in FIRS could be limited to construction on shallow water.

Water depth 0 and 1 have the original cost to clear, higher depths have the cost to clear multiplied by the square of depth, so clearing the max depth of 15 (represented in land area information window as 300 m) has a cost of around £1.7 million at default 'low' construction costs, while clearing a water tile at the coast costs £7,524.
Rivers are soft limited to a depth of 3 (60 m), and canals are always built at depth 0 (10 m).

Water tiles can "erode" over time to even out the depth gradient from the shore. This is an option that can be disabled, or the frequency can be selected.

Not in scope

  • Depth-aware bridges. Doing an overhaul of bridges is a large project in itself and is probably best tackled in a later patch.
  • Multiple variations of water tiles. Having more than one tile graphics for each depth level could reduce visual repetition, but is a large project in itself.
  • Adding deep water tiles to the original baseset, although it should probably be done before a release is made with this patch included.

To do

  • Canals built over rivers should probably inherit the depth of the river.
  • Evening out of depth and depth gradient should be more controlled and slower -- maybe a game setting to control the speed or entirely disable it.
  • Need a way for GS and AI to query depth.
  • Need a way for NewGRF to query depth, especially for industries constructing on water.
  • Perhaps change the ship "on river/canal" speed limit to count as "on shallow water" instead.
  • Would be neat to have different water tiles for different depths (perhaps darker blue).
  • Configurable cost of modifying deep water tiles.
  • Need a test case of rivers/canals GRF using new variable for depth.
  • Find a way to make ground sprites below oil rig be depth aware.

Unresolved questions

  • Should industry tiles and station tiles also have space to store water depth? Right now, industry tiles do not store water depth, instead the industry itself stores the min and max water depths it was on when it was constructed. Stations (docks, buoys, oil rig stations) store nothing and instead search for a nearby water tile whose depth is assumed.
  • What should the default setting for difficulty.water_clearing_cost_exponent be? Is the "basic" category appropriate?
  • What should the default setting for difficulty.water_depth_erosion_speed be? Is the "advanced" category appropriate?
  • What values should those settings be given when savegames are converted?

NewGRF docs that need to be updated with this PR

  • Action 5 adds feature 0x19 to define deep water terrain sprites. Requires an array of sprite offsets as "other data" mapping depth levels to sprites.
  • Canal/river feature callbacks add variable 0x84 to query the water depth of the current tile.
  • Industry placement callback has two new error returns available, 0x409 and 0x40A for respectively "water too deep" and "water too shallow".
  • Industry callbacks add two new variables, 0xAD and 0xAE, to get the minimum and maximum water depth on the site the industry was/is going to be constructed.
  • Ships feature callbacks add variable 0xE6 to get water depth of current tile.
@James103
Copy link
Contributor

James103 commented Jan 11, 2020

I have a couple of points to make regarding customizing this feature:

  1. Could you add a way to disable, lessen, (or even increase) the cost multiplier of clearing water tiles?
  2. Could you add a way to disable the water depth check when building ship depots?
  3. Could you add a way to reduce the max water depth from level 15 (even all the way to 0 to disable this feature)?
@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 11, 2020

@James103 Point 1 and 2, maybe. For cost of clearing water, the setting could be between "no multiplier", "more expensive by depth" and "much more expensive by depth" with the last being my current square-of-depth multiplier.

Point 3 I don't think is a good idea. NewGRF support will be more complex if depth levels can be arbitrarily limited.

@LordAro
Copy link
Member

LordAro commented Jan 11, 2020

We cannot add options to disable every single change to the game.

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 11, 2020

This, along with the earlier "neutral stations only serve their owning industry" change are support for changing the long-standing TT tradition of filling in the ocean. With deep ocean being expensive to fill in, building bridges crossing large water bodies as short segments with intermediate islands for signals, is also much less viable, making transfer to ships or longer routes around the water body more attractive. In all, I think this change will make for much more interesting gameplay.

@sheepo99
Copy link

sheepo99 commented Jan 11, 2020

This, along with the earlier "neutral stations only serve their owning industry" change are support for changing the long-standing TT tradition of filling in the ocean. With deep ocean being expensive to fill in, building bridges crossing large water bodies as short segments with intermediate islands for signals, is also much less viable, making transfer to ships or longer routes around the water body more attractive. In all, I think this change will make for much more interesting gameplay.

I sense this would also imply the need for more vessel types for navigating diff depths and balancing ships overall, meaning new vehicle graphics of some sort would be necessary as well. It would also make canals for bigger ships much more expensive, privileging small-ship networks for inland canals. So many interesting options!

@James103
Copy link
Contributor

James103 commented Jan 11, 2020

With this change, clearing a large enough body of water will cause the total cost to roll past 64 bits, resulting in an overflow and possibly gaining money.

  • Cost to clear water tile of depth 0: 7,500
  • Cost multiplier from depth 15: x225
  • Maximum map size: x16,777,216 tiles
  • Max NewGRF cost multiplier: x65,536
  • Max inflation multiplier: x887
  • Total Cost to clear All-Water 4k map: 1,855,425,871,872,000,000
  • Total Cost to clear All-Water 4k map after inflation: 1,647,156,981,589,051,266,457
  • Maximum Tiles before 64-bit Overflow (after inflation): 93,945 tiles
  • MAX_INT64 (signed): 9,223,372,036,854,775,807

Therefore: there is a risk of a 64-bit overflow after this change, but it only applies when estimating the cost in the Scenario Editor. If the cost multiplier per unit of depth were just 0.5-1 higher than what it is now (depth^2.5 or depth^3), then said risk would become real as the number of tiles required to be demolished to trigger a 64-bit overflow would now fit within the maximum tile clearing limits.

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 11, 2020

I'm glad to hear that the cost of filling in the ocean is finally getting reasonable. I don't think the above shows a problem with this PR, rather if an underflow can occur that's a bug in the Money class. Also, wouldn't you generally hit the landscaping limit throttle before the underflow?

@nielsmh nielsmh force-pushed the nielsmh:deepwater branch from ab8fba2 to ebc7a9c Jan 12, 2020
src/water_cmd.cpp Outdated Show resolved Hide resolved
@James103
Copy link
Contributor

James103 commented Jan 12, 2020

Looks like the regression tests are failing (as of 8f59276). Can you please check where the regression tests are failing and why they are failing?

@SamuXarick
Copy link
Contributor

SamuXarick commented Jan 12, 2020

they fail because it cannot place a ship depot

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 12, 2020

What @SamuXarick says. I'm looking into a bug with how water depth is restored when a ship depot gets cleared, the problem is that it doesn't. (So you can glitch depth by filling a deep area with depots, clearing them, and then filling in the sea to avoid a few millions in cost.)

@James103
Copy link
Contributor

James103 commented Jan 12, 2020

The regression tests are still failing. Why? Is it because of not enough money? Additionally, the macOS regression test timed out after 1 hour. The logs say that the regression test AI "script died unexpectedly".

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 12, 2020

The regression is failing because the program crashes. The program crashes because of an assertion error. The assertion error is that a tile that was expected to be water was not water. The assertion happens during destruction of a ship depot.

The macOS build hangs because it does not handle crashes in the regressions tests.

@nielsmh nielsmh force-pushed the nielsmh:deepwater branch from 3eb2e04 to d1fafce Jan 12, 2020
@floodious
Copy link

floodious commented Jan 14, 2020

With this change, clearing a large enough body of water will cause the total cost to roll past 64 bits, resulting in an overflow and possibly gaining money.

There are multiple ways to handle such a situation. One is to brute-force compute the sum by using a bignum accumulator that grows by adding additional words as needed. This requires an additional check for wrapping on each operation, once per additional N-bit word. Such bignum implementations already exist and their costs are known.

Alternatively it's possible to ensure within the known context that wrapping can never occur. Noting that only nonlinear operations like square/log/exp are noncommutative while multiplication is; if the sum is minimized by computing the sum of the squares of the height multiplier before computing the product:

(depth 0 == x1, therefore 15 == x16 ?)
Maximum cost multiplier from depth 16: 16^2 == 256 (2^8)
Maximum map size: 4096^2 == 16777216 tiles (2^24)
Maximum multiplicand/sum: 256 x 16777216 = 4294967296 (== 2^32)

This sum can then be used to compute the product via the bignum code (128 bit or otherwise) including the coefficient for Cost x Inflation. This means the overhead of the bignum product would only apply to a single step at the end of the summation stage and the sum itself would fit well within a 64 bit accumulator.

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 14, 2020

OTTD already uses a Money type which is supposed to be safe against overflow (by saturating at INT64_MAX), but does not check for underflow. That needs to be fixed in src/core/overflowsafe_type.hpp and is not a discussion for this PR.

@floodious
Copy link

floodious commented Jan 14, 2020

Right, that was going to be my first suggestion to use saturating arithmetic, but that can't possibly compute the correct sum and it's generally less efficient without hardware support. So I thought, the only way to actually compute the accurate sum+product is to use a multi-step process where the sum and product are handled separately with adequate precision for each.

@floodious
Copy link

floodious commented Jan 14, 2020

Regarding the actual cost scaling:

squares

Assuming the surrounding vertices are equal height, at depth:

  1. = 1
  2. = 3^2 + 1 == 10
  3. = 5^2 + 3^2 + 1 == 35
  4. = 7^2 + 5^2 + 3^2 + 1 == 84

To get the accurate scaling I can't see any way other than actually iterating through every vertex and summing the difference in height. x^2 seems ... I'm blanking out on the terms here, but what was the rationale for x-squared?

(Blanking out: the difference isn't merely polynomial but rather ... ? Making application of a polynomial "severely underestimate the growth of the cost with increasing magnitude of x." = word I'm looking for.)

Sure took me long enough to bother to remember: Factorial, hyper-(exponentiation, ...), tetration, pentation, ...

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 14, 2020

I don't think having a fully correct arithmetic for pathological cases that are only possible on gigantic maps with inflation and landscaping costs maxed out is important. As long as it results in the player being refused the action because they would go into negative it's good enough. Even in the extreme case of the player having INT64_MAX cash on hand, and performing an action that should cost more than INT64_MAX, the action succeeding and the playing ending up at zero is also acceptable.

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 14, 2020

My reason for using quadratic growth on sea clearing cost is that it's easy to implement and does a lot to make filling in the sea unattractive, not that it models anything resembling the real world.

@floodious
Copy link

floodious commented Jan 14, 2020

I'd need to look at whether the existing landscaping cost computation uses correct iteration and computation of height differences given the "1 per tile" rule. Computing the accurate cost if it isn't yet implemented may very well "solve" the need for limitations on landscaping by making them bear more realistic cost and scaling. On the other hand, it may also make more than very minor tweaks very cost prohibitive as they become way higher order than a polynomial.

edit to add:
I suppose the volume of a rectangular pyramid is the proper volume. In the square case the area would indeed be quadratic with the product of height in addition to that if I'm not mistaken, shouldn't the accurate maximum value then be height^2*height/3 ? If so my thinking x^^n was wrong since it's only (n*2+1)^2+((n-1)*2+1)^2+... == n^2*n/3, apparently.
Then:

  1. = 1/3, 2. = 8/3, 3. = 27/3, 4. = 64/3
    So n^2 would be ... (sub- ... that word I'm looking for?) but only very slightly so and close to accurate.

edit again: (or isn't that then supposed to be n^3/3?)
Still can't recall this one: hypographic (underestimating) vs. epigraphic (overestimating) (?).

@RoqueDeicide
Copy link

RoqueDeicide commented Jan 14, 2020

This change feels wrong to me, unless it comes with an option to make clearing of water tiles cost the same amount regardless of depth. This would allow marriage of new mechanics, like limiting ability to build depots, with classic gameplay without turning the game into one of those "hard-mode" patchpacks (I don't play on the latter for a reason).

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 15, 2020

Here's two NewGRFs with deep water sprites, drawn by TT-Forums users wallyweb and Andrew350, and the NFO files adjusted by me.
water-depth-newgrfs.zip

The WaterCycleTest GRF is 8bpp and based on original baseset, uses palette animation. The darkwater GRF is 32bpp and based on OpenGFX, does not have palette animation but covers all 16 depth levels.

@nielsmh nielsmh force-pushed the nielsmh:deepwater branch from 990a4eb to 69ae305 Jan 16, 2020
@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 16, 2020

Added a NewGRF variable to get the water depth for industry callbacks, although I'm not sure this is the best way to do it. The variable will probably not be any use after the industry's construction since the tiles are now all industry instead of water, so it should perhaps only be available during the location check callback, in which case it could override one of the variable numbers currently occupied by various cargo statuses that are useless during location check.

nielsmh added 27 commits Jan 12, 2020
…RF purposes

This is to avoid risks of desync if the depth changes while a ship is on a tile, and one client
considers the old depth and another client (which arrived just now) considers the new depth.
@nielsmh nielsmh force-pushed the nielsmh:deepwater branch from 7b363e7 to a7bfef2 Jun 28, 2020
@nielsmh
Copy link
Contributor Author

nielsmh commented Jun 28, 2020

Perhaps I should also add a GSTile::SetWaterDepth function to let GS modify the depth, but that will either require a new command or have it piggyback on an existing landscaping command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.