FS#983 - High Sea Level Patch, Done

Attached to Project: OpenTTD
Opened by BoekaBart (boekabart) - Tuesday, 03 July 2007, 14:00 GMT
Type Patch
Category Core
Status New
Assigned To No-one
Operating System All
Severity Low
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


Hi guys, I think the highsealevel patch is pretty much finished now that it has a user-friendly gui for 'new games' ( Please seriously consider it, it'll be a bitch to update again after the 6 weeks I will be away for... ;) Have a nice summer and TTY in august!
This task depends upon

Comment by BoekaBart (boekabart) - Friday, 05 October 2007, 10:36 GMT
So it wasn't that much of a bitch to update it after all :)

It has been in ChrisIn for a small while now, no complaints... So, hereby an update and let's see if this can go into trunk before we split of 0.6.0
Note that after this, adding rivers is a breeze - if the gfx are ever made.
Comment by Remko Bijker (Rubidium) - Friday, 05 October 2007, 22:25 GMT
I'd move the assertions from IsCoast/IsSea to GetWaterTileType.

+ // This used to check TileSlope == flat too.
+ // But none of the uses actually requires flat water, just (clear)Water (canal, river, sea)
Use the /* ... */ style here.

+extern uint8 _map_sealevel;
+extern uint GetSlopeZ_WaterSurface(TileIndex tile, uint x, uint y);
+extern Slope GetSlopeTileh_WaterSurface(TileIndex tile, Slope tileh);
Those are not map accessors. Probably best to move them to water.h

+/* water_cmd.cpp */
+extern uint8 _map_sealevel;
Kind of duplicate, just include water.h in misc.cpp

+ switch(GetTileType(tile))
+ {
the { should be on the line with switch, the case should be indented one more level.

+extern uint8 _map_sealevel;
+static inline bool IsSeaBuoyTile(TileIndex t)
I'd move the extern inside the function, as it is only intended to be used by that function.

+/* water_cmd.cpp */
+extern uint8 _map_sealevel;
+extern void LowerSeaLevel();
+extern void RaiseSeaLevel();
Don't we use include files to keep that?
Comment by BoekaBart (boekabart) - Monday, 08 October 2007, 09:32 GMT
Thanks for the comments - I think I've got them all in this new version.
Added water.h ( not in projects, but added in source.list )
Comment by BoekaBart (boekabart) - Monday, 08 October 2007, 11:43 GMT
All by myself without any feedback from TrueBrainLight, I fixed some minor coding style issues.
Comment by BoekaBart (boekabart) - Monday, 08 October 2007, 15:03 GMT
Maybe, there will come a day where there will be no comments left to make.
* Changed the order of check IsSea, and the comment there, to be better understandable.
* Fixed a typo (wrong '(' location) made in the previous 2 versions.
Comment by BoekaBart (boekabart) - Tuesday, 09 October 2007, 21:56 GMT
'svn up' (savegame version to 81)
Comment by BoekaBart (boekabart) - Wednesday, 10 October 2007, 08:52 GMT
'svn up' conflicted once again.
Comment by BoekaBart (boekabart) - Tuesday, 16 October 2007, 16:30 GMT
I've split the diff in 3 parts - but with svn I can't really make 3 diffs that come one-after-the-other, so here's just the first one.
It fixes the fact that OpenTTD still supports above-sea level canals with owner WATER - the fix done by Rubidium in r10230 (Implemented IsCanal() and use it to disallow terraforming) doesn't work for those canals. For an example, load opntitle.dat as a savegame and terraform around the canals there.
This diff fixes that and uses the IsCanal (and some other accessors) in all places where IsCanal needs to be checked.

Next step: use _map_sealevel for all 'water level' checks, and store it in the savegame - but no sealevel modification UI/functions yet.
Last step: Add Sealevel GUIs.
Comment by BoekaBart (boekabart) - Tuesday, 16 October 2007, 22:25 GMT
OK, step 1 in trunk (r11276)

Hereby, the biggest step:
* the introduction of _map_sealevel in water.h
* using this value everywhere where relevant
* storing it in savegames
* modified the flooding algorithm to work with now-possible situations (but behave the same when water is at 0, and behave visually the same ate higher sealevels.
* ships follow the water level, not the sea floor
* industries, bouys, docks and depots must be built on shallow water (added error string)
* towns won't terraform away dikes that will cause floods
* draw steep slopes with one corner under water with the right 'shore' equivalent
* draw an extra sea floor sprite during flooding if not drawing it would cause a 'hole in the map'-glitch.
I consider all those steps essential for the move towards mainly deep water, but adding the variable is imho an important step towards implementing rivers in a nice way.

Not in here yet, but ready to go in in step 3 or 4:
* All GUIs to actually raise the sea level (scenario editor and during random map generation)
* Transparent water option (draws the sea floor in the form of a 'dark blue selection grid' where it's deeper than 0)

Another option for step 3 or 4:
* Rivers code (as in TTDPatch currently - without drawing borders)
Comment by Remko Bijker (Rubidium) - Saturday, 20 October 2007, 08:40 GMT
+ return IsBuoyTile(t) && IsTileOwner(t, OWNER_WATER) && TileHeight(t) <= _map_sealevel;
This is going to cause trouble (I think) with canals that are below sealevel (in polders).

+ if (IsTileType(target, MP_WATER)) {
+ if (IsSea(target)) return;
+ }
The following is in my opinion "easier" to read (there are several similar cases)
if (IsTileType(target, MP_WATER) && IsSea(target)) return;

NOTE: to myself... run project/generate before committing ;)