completely shore

[Digitalfox]

So BigBB, can we assume your patch is finished ( besides The fix for the railway on corner-shore-tiles )??

Keep the good work..

[boekabart]

So BigBB, can we assume your patch is finished ( besides The fix for the railway on corner-shore-tiles )??

Keep the good work..

Almost I'd say, but until the flooding change isn't a patch option I don't consider it finished....

[George]

So BigBB, can we assume your patch is finished ( besides The fix for the railway on corner-shore-tiles )??

Keep the good work..

Almost I'd say, but until the flooding change isn't a patch option I don't consider it finished....

And would it work under industry on shore? Try tourists resort

[BigBB]

So BigBB, can we assume your patch is finished ( besides The fix for the railway on corner-shore-tiles )??

Keep the good work..

Thanks, but first bug searching...

Almost I'd say, but until the flooding change isn't a patch option I don't consider it finished....

I don't think so. And I think that the reason "it is not in TTo" is not enough. So I only make a switch, if a dev tell me I must .

Regards
BigBB

Edit:

And would it work under industry on shore? Try tourists resort

Okay, I test it ....

[boekabart]

And I think that the reason "it is not in TTo" is not enough.

As Far As I Know, it's normal and good practice to make all changes that change the TTD default behaviour are toggle-able as patch options.

Now, on to the patch itself

Code: Select all

+void DrawShoreTile(const TileInfo *ti);

What is this doing in functions.h? Some water_*.h would be better.

Code: Select all

+void DrawShoreTile(const TileInfo *ti)

What is this doing in clear_cmd.cpp?

Code: Select all

const bool IsFloodableTile(

What does that const mean?

The IsFloodableTile (especially with the invert boolean) is a little vague in my opinion - although the idea of adding such a function is very good.

I would rather see a bit added to bridge tiles (they have plenty free!) to indicate that they are water (then they can actually flood too!), this gets rid of the flash-flooding under bridges and the 'tile jumping' in the code. You won't need the invert bit either, then. The IsFloodableTile function could look like:

Code: Select all

bool DoesTileFloodTile( TileIndex some_wet_tile, TileIndex some_dry_tile )

or

Code: Select all

bool DoesTileFloodInThisDirection( TileIndex some_wet_tile, DiagDirection dir )

...which can do all the necessary checks, including the patch option (I won't stop about it) whether canyons should flood.

Code: Select all

+	uint tile_height; // the tile height (0-15)
+	Slope slope; // the slope type of the tile

Can be moved into the for loop.

Last one: Why did you replace the const array _water_shore_sprites by the switch/case statement, rather than just extending it with the extra SPR_SHORE tiles?

Edit: Really last one: Why is the savegame version increased?

[boekabart]

Found a bug (that I expected after reading the code).

...this bridge used to get washed away by water...

[boekabart]

Another 'bug' - without newwater, tiles can be shore without the player noticing (after careless clearing), and this invisible shore corner does flood its neighbours.

[BigBB]

Code: Select all

+void DrawShoreTile(const TileInfo *ti);

What is this doing in functions.h? Some water_*.h would be better.

Code: Select all

+void DrawShoreTile(const TileInfo *ti)

What is this doing in clear_cmd.cpp?

I write it there, because DrawClearLandTile() is there too. But it can be moved .

The IsFloodableTile (especially with the invert boolean) is a little vague in my opinion - although the idea of adding such a function is very good.

Okay, I will it document more....

I would rather see a bit added to bridge tiles (they have plenty free!) to indicate that they are water (then they can actually flood too!), this gets rid of the flash-flooding under bridges and the 'tile jumping' in the code. You won't need the invert bit either, then. The IsFloodableTile function could look like: [...]

Okay, thanks. I have a look on it (I knew that I forgot an option... ).

Why did you replace the const array _water_shore_sprites by the switch/case statement, rather than just extending it with the extra SPR_SHORE tiles?

I don't need this array in water_cmd.cpp, so it musn't extended.

Why is the savegame version increased?

Simple, try to load a game with corner shores in an OTTD which can not interpret them.....

-BigBB

[boekabart]

I've taken the liberty to implement the bridges with their own bit to indicate they are on a shore - this cleans up the code quite a bit.

Still rough since I'm in a hurry now!

[Ben_K]

BigBB, massive repsect to you for your hard work and efforts on this project. Also, congratulations with the 'advanced faces' patch added to trunk.

Keep up the great work!

[BigBB]

New version (1.4)

...this bridge used to get washed away by water...

fixed

Another 'bug' - without newwater, tiles can be shore without the player noticing (after careless clearing), and this invisible shore corner does flood its neighbours.

corner shores now only floods if a NewWater grf loaded (but if you deactivate this grf, the corner shores will not remove)

And would it work under industry on shore? Try tourists resort

I don't change something with NewIndustries, so the tourist center don't change shore.


I'm not satisfied with IsFloodableTile(), but atm I have no good idea...

-BigBB

[George]

New version (1.4)

And would it work under industry on shore? Try tourists resort

I don't change something with NewIndustries, so the tourist center don't change shore.

And what to do?

[BigBB]

And what to do?

Is there no NewGRF option to do that?

[George]

And what to do?

Is there no NewGRF option to do that?

There is no way to access new graphics, like snowed trees in temperate, tram track, electrified rails and so on. And new shores too
So, the patch should test, if the industry tile on shore uses default land tile (with or without shore) by checking sprite number for example and draw new slope instead.

[BigBB]

New version (1.5):
- now I'm satisfied with IsFloodableTile()
- bridgeheads on shore can now flood too (thanks boekabart)


-BigBB

Edit:
forget something...
new version (1.5.1):
-fix industry shore tiles

There is no way to access new graphics, like snowed trees in temperate, tram track, electrified rails and so on. And new shores too
So, the patch should test, if the industry tile on shore uses default land tile (with or without shore) by checking sprite number for example and draw new slope instead.

Okay, then I fix it

industry-shore.png (36.2 KiB) Viewed 11934 times

[frosch]

Hello.

There is a critical problem with the Canyon-flooding. It breaks the nice "Canal on sea level"-feature:

"Canal on sea level"-feature is broken by the Canyon-Flooding.

CanalOnSeaLevel.png (15.73 KiB) Viewed 5255 times

In trunk this feature enables you to build things on see level without getting flooded.

As you already fixed the ground sprites of docks: Perhaps you want to fix the ground sprites of docks at canals:

Docks at canals draw shore graphics. (Also a trunk bug)

DockAtCanal.png (9.92 KiB) Viewed 5251 times

This thread summaries a lot bugs around water. So I add another one to the list. (Though I do not expect, that your patch will fix everything.)

Also a trunk bug: After removing a depot from the water, the canal dikes are not updated. This also happens when flooding near "canal on sea level".

DikesUpdate.png (9.47 KiB) Viewed 5252 times

There are a lot situations, where the dikes of canals are not updated. E.g. removing of depots, flooding of neighboured tiles, ...

[boekabart]

I think that the canals-to-keep-the-sea-out feature shouldn't work with newwater anyway, since newwater actually looks like it's about halfway the tileheight instead of at the bottom, like normal water and canals.

In general, I think for new water the flooding should behave differently in 3 area's.
1) canyon flooding should be 'on'
2) corner flooding should be 'on'
3) canal flooding should be 'on'
This behavior should be controlled either by a new grf action, of by a patch setting - not by the fact that a water replacement set is loaded (eg. mars 'water' shouldn't behave 'new').

This could be a separate patch, apart from the patch that allows BridgeHeads On Shores to have 'coast'.

[boekabart]

I made a patch that only does the flooding of the bridge heads, I guess this is a feature that comes in handy no matter what happens with the rest of the water discussion.

It should work fine with both newwater and oldwater.

Credits and thanks to BigBB for the idea; some of his code/comments may also be in there

[BigBB]

New version (1.6):
- make a combination of the old flooding mechanism and my new
- removed canyon flooding till a NewWater NewGRF exist with spirits for SLOPE_EW and SLOPE_NS
- removed code duplications

I think the patch is now complete.

There is a critical problem with the Canyon-flooding. It breaks the nice "Canal on sea level"-feature:
In trunk this feature enables you to build things on see level without getting flooded.

I'm not sure if it is a featur or a bug , but is fixed. Your other points will I fix in a seperate patch, I think this one have enough.


-BigBB

[belugas]

This patch is getting better all the time