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

High Sea Level Patch, Done #983

Closed
DorpsGek opened this issue Jul 3, 2007 · 11 comments
Closed

High Sea Level Patch, Done #983

DorpsGek opened this issue Jul 3, 2007 · 11 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Jul 3, 2007

boekabart opened the ticket and wrote:

Hi guys, I think the highsealevel patch is pretty much finished now that it has a user-friendly gui for 'new games' (http://www.tt-forums.net/viewtopic.php?p=603836# 603836). 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!

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/983
@DorpsGek
Copy link
Member Author

DorpsGek commented Oct 5, 2007

boekabart wrote:

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.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2324

@DorpsGek
Copy link
Member Author

DorpsGek commented Oct 6, 2007

Rubidium wrote:

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))
+ {
+ case MP_INDUSTRY:
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?


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2339

@DorpsGek
Copy link
Member Author

DorpsGek commented Oct 8, 2007

boekabart wrote:

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 )

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2358

@DorpsGek
Copy link
Member Author

DorpsGek commented Oct 8, 2007

boekabart wrote:

All by myself without any feedback from TrueBrainLight, I fixed some minor coding style issues.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2359

@DorpsGek
Copy link
Member Author

DorpsGek commented Oct 8, 2007

boekabart wrote:

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.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2360

@DorpsGek
Copy link
Member Author

DorpsGek commented Oct 9, 2007

boekabart wrote:

'svn up' (savegame version to 81)

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2380

@DorpsGek
Copy link
Member Author

boekabart wrote:

'svn up' conflicted once again.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2383

@DorpsGek
Copy link
Member Author

boekabart wrote:

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.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2435

@DorpsGek
Copy link
Member Author

boekabart wrote:

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)

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2440

@DorpsGek
Copy link
Member Author

Rubidium wrote:

+ 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 ;)


This comment was imported from FlySpray: https://bugs.openttd.org/task/983#comment2469

@DorpsGek
Copy link
Member Author

andythenorth closed the ticket.

Reason for closing: Won't implement

Mass closure of patch tickets with no commentary for >5 years. Goal is to reduce patch queue as an experiment to see if it aids faster reviewing and rejection/acceptance (it may not). If this offends you and the patch is maintained and compiles with current trunk, discuss with andythenorth in irc. (andythenorth has no ability to review patches but can re-open tickets). Also, Tsunamis! ;)


This comment was imported from FlySpray: https://bugs.openttd.org/task/983

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Development

No branches or pull requests

1 participant