Page 2 of 4

Re: completely shore

Posted: 17 Oct 2007 13:19
by Digitalfox
So BigBB, can we assume your patch is finished ( besides The fix for the railway on corner-shore-tiles )??

Keep the good work.. :D

Re: completely shore

Posted: 17 Oct 2007 13:40
by boekabart
Digitalfox wrote:So BigBB, can we assume your patch is finished ( besides The fix for the railway on corner-shore-tiles )??

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

Re: completely shore

Posted: 17 Oct 2007 13:56
by George
boekabart wrote:
Digitalfox wrote:So BigBB, can we assume your patch is finished ( besides The fix for the railway on corner-shore-tiles )??

Keep the good work.. :D
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 ;)

Re: completely shore

Posted: 17 Oct 2007 13:56
by BigBB
Digitalfox wrote:So BigBB, can we assume your patch is finished ( besides The fix for the railway on corner-shore-tiles )??

Keep the good work.. :D
Thanks, but first bug searching... :)
boekabart wrote: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:
George wrote:And would it work under industry on shore? Try tourists resort ;)
Okay, I test it ....

Re: completely shore

Posted: 17 Oct 2007 14:03
by boekabart
BigBB wrote: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?

Re: completely shore

Posted: 17 Oct 2007 14:16
by boekabart
Found a bug (that I expected after reading the code).

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

Re: completely shore

Posted: 17 Oct 2007 14:20
by 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.

Re: completely shore

Posted: 17 Oct 2007 14:25
by BigBB
boekabart wrote:

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 :).
boekabart wrote: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....
boekabart wrote: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... :) ).
boekabart wrote: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.
boekabart wrote:Why is the savegame version increased?
Simple, try to load a game with corner shores in an OTTD which can not interpret them.....

-BigBB

Re: completely shore

Posted: 17 Oct 2007 15:49
by 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!

Re: completely shore

Posted: 17 Oct 2007 16:53
by 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!

Re: completely shore

Posted: 18 Oct 2007 02:47
by BigBB
New version (1.4)
boekabart wrote:...this bridge used to get washed away by water...
fixed
boekabart wrote: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)
George wrote: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

Re: completely shore

Posted: 18 Oct 2007 04:06
by George
BigBB wrote:New version (1.4)
George wrote: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?

Re: completely shore

Posted: 18 Oct 2007 08:53
by BigBB
George wrote:And what to do?
Is there no NewGRF option to do that?

Re: completely shore

Posted: 18 Oct 2007 09:17
by George
BigBB wrote:
George wrote: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.

Re: completely shore

Posted: 18 Oct 2007 11:05
by 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
George wrote: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
industry-shore.png (36.2 KiB) Viewed 11940 times

Re: completely shore

Posted: 18 Oct 2007 14:42
by 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.
"Canal on sea level"-feature is broken by the Canyon-Flooding.
CanalOnSeaLevel.png (15.73 KiB) Viewed 5261 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)
Docks at canals draw shore graphics. (Also a trunk bug)
DockAtCanal.png (9.92 KiB) Viewed 5257 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".
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 5258 times
There are a lot situations, where the dikes of canals are not updated. E.g. removing of depots, flooding of neighboured tiles, ...

Re: completely shore

Posted: 18 Oct 2007 16:33
by 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'.

Re: completely shore

Posted: 18 Oct 2007 17:12
by 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 :)

Re: completely shore

Posted: 18 Oct 2007 18:56
by 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.
frosch wrote: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

Re: completely shore

Posted: 26 Oct 2007 19:24
by belugas
This patch is getting better all the time :D