Skip to content

Diagonal Rail#528

Closed
khonkhortisan wants to merge 1 commit intoluanti-org:masterfrom
khonkhortisan:diagonal_rail
Closed

Diagonal Rail#528
khonkhortisan wants to merge 1 commit intoluanti-org:masterfrom
khonkhortisan:diagonal_rail

Conversation

@khonkhortisan
Copy link
Contributor

Abstract

This self-referencing pull request adds diagonal rails.

Proof of Concept

minetestdiagonalrail8
Gif
minetestdiagonalrail9

Images

They are just the same as regular rail except that they use two more images:

-       tiles = {"default_rail.png", "default_rail_curved.png", "default_rail_t_junction.png", "default_rail_crossing.png"},
+       tiles = {"default_rail.png", "default_rail_curved.png", "default_rail_t_junction.png", "default_rail_crossing.png", "default_rail_diagonal.png", "default_rail_diagonal_end.png"},

If this commit is used without the supplementary images, diagonals will appear as crossings. That is why I include child pull requests in three places [see Merging]
End diagonals aren't simply rotated to turn left like curves/diagonals are, they must be flipped so the end is on the other side.
If you use raillike nodes for roofing, this probably won't affect you, unless you use unusual non-rectangular roofing.

Pseudo-rules

A curve will turn diagonal if there are two or more curves in the same diagonal direction (no U-turns)
A rail will end diagonal if it is between a diagonal and a non-curve – unless it is next to the end of the line, in which case that will end diagonal (so non-curve includes non-rail)
Height differences don't apply, as they always create straight rail.

Intersections

There are no diagonal intersections. To do this, one would first have know how to keep two adjacent rail lines from intersecting. They aren't needed at this time.

How it works

It (a rail) reads its neighbors to know how many adjacencies there are, then based on that decides its image. Diagonals are based on more than just the immediate neighbors, so a recursive function is used. The top-level recursion is for a rail, and has its neighbors. The middle level gets its neighbors' neighbors. Using the neighbors' neighbors, a rail can decide if its neighbor is indeed curved, and become diagonal (as its neighbor does the same when the top-level function gets called at that position). But it goes one level further; there are three levels of recursion. The very end of a diagonal rail line (1) needs to know that its neighbor (2) is curved, and that its neighbor (3) is also curved (so it (1) knows that its neighbor (2) would've been an end diagonal had it (1) not been the end of a line). That is the end of the recursion.

Merging

May not automatically merge with connect_to_raillike but will merge manually (this moves the code it changes)
Merge around the same time as these child pull requests: minetest_game, carts, moreores

Problems

  1. Occasionally a rail will appear straight after restarting my singleplayer game. Tapping the node makes it behave. I do not believe my code is part of this problem.
  2. Coding style. If you see something you would like me to change, tell me (I'll git commit --amend; git push --force)
  3. Usefulness. This is a graphical change. If you use rails, this will make them prettier, but any potential train/cart-like vehicles that travel along them may (will) still zig-zag as if there are left/right alternating curves. The visual appearance of a straight diagonal rail line may encourage a vehicle designer to let the vehicle travel diagonally, using the same rules in lua form.
  4. I didn't care to look whether this was a client or server change.

@Jordach
Copy link
Contributor

Jordach commented Mar 6, 2013

This MUST BE MERGED.

On Wednesday, March 6, 2013, khonkhortisan wrote:

Abstract

This self-referencing pull request adds diagonal rails.
Proof of Concept

[image: minetestdiagonalrail8]https://f.cloud.github.com/assets/1996449/226181/69c78826-861f-11e2-8edd-21c295b12fa2.png
Gifhttps://f.cloud.github.com/assets/1996449/226179/698502bc-861f-11e2-96fd-941008bb496c.gif
[image: minetestdiagonalrail9]https://f.cloud.github.com/assets/1996449/226180/69c593e0-861f-11e2-9a62-f73643ba5dc0.png
Images

They are just the same as regular rail except that they use two more
images:

  •   tiles = {"default_rail.png", "default_rail_curved.png", "default_rail_t_junction.png", "default_rail_crossing.png"},
    
  •   tiles = {"default_rail.png", "default_rail_curved.png", "default_rail_t_junction.png", "default_rail_crossing.png", "default_rail_diagonal.png", "default_rail_diagonal_end.png"},
    

If this commit is used without the supplementary images, diagonals will
appear as crossings. That is why I include child pull requests in three
places (minetest_game, carts, moreores)
End diagonals aren't simply rotated to turn left like curves/diagonals
are, they must be flipped so the end is on the other side.
If you use raillike nodes for roofing, this probably won't affect you,
unless you use unusual non-rectangular roofing.
Pseudo-rules

A curve will turn diagonal if there are two or more curves in the same
diagonal direction (no U-turns)
A rail will end diagonal if it is between a diagonal and a non-curve –
unless it is next to the end of the line, in which case that will end
diagonal (so non-curve includes non-rail)
Height differences don't apply, as they always create straight rail.
Intersections

There are no diagonal intersections. To do this, one would first have know
how to keep two adjacent rail lines from intersecting. They aren't needed
at this time.
How it works

It (a rail) reads its neighbors to know how many adjacencies there are,
then based on that decides its image. Diagonals are based on more than just
the immediate neighbors, so a recursive function is used. The top-level
recursion is for a rail, and has its neighbors. The middle level gets its
neighbors' neighbors. Using the neighbors' neighbors, a rail can decide if
its neighbor is indeed curved, and become diagonal (as its neighbor does
the same when the top-level function gets called at that position). But it
goes one level further; there are three levels of recursion. The very end
of a diagonal rail line (1) needs to know that its neighbor (2) is curved,
and that its neighbor (3) is also curved (so it (1) knows that its
neighbor (2) would've been an end diagonal had it (1) not been the end of a
line). That is the end of the recursion.
Merging

May not automatically merge with #473https://github.com/minetest/minetest/issues/473but will merge manually.
Merge around the same time as these child pull requests: minetest_gamehttps://github.com/luanti-org/minetest_game/pull/135,

carts PilzAdam/carts#8, moreoreshttps://github.com/khonkhortisan/calinou_mods/pull/1

You can merge this Pull Request by running

git pull https://github.com/khonkhortisan/minetest diagonal_rail

Or view, comment on, or merge it at:

#528
Commit Summary

  • Diagonal Rail

File Changes

Patch Links:

@PilzAdam
Copy link
Contributor

PilzAdam commented Mar 6, 2013

Isnt this against the voxel idea of Minetest?

@khonkhortisan
Copy link
Contributor Author

Maybe slightly. In the games Transport Tycoon Deluxe/OpenTTD/Simutrans, even though the world is divided into squares, it's still able to have diagonal rails. Those rails are slightly more complicated, you can have two diagonal lines in the same square. I'm using this as an example of how diagonal rails work just fine in a cubical world.

@PilzAdam
Copy link
Contributor

PilzAdam commented Mar 6, 2013

This would also complicate the movement of carts (if they want to move diagonal too).

@Uberi
Copy link
Contributor

Uberi commented Mar 6, 2013

I think the rail texture looks a bit too "thin" - the wheel spacing seems to be slightly different from the default rail spacing.

@khonkhortisan
Copy link
Contributor Author

Should carts treat diagonal rail the same as curved rail and move diagonally on both of them? Or should it still move like a square on curves? Or should it actually arc on a curve yet move straight on a diagonal?

The narrower rail can be made the same width as the wider rail either by drawing it larger than the size of one node, or making the regular rail narrower.

@kwolekr
Copy link
Contributor

kwolekr commented Mar 17, 2013

I would like to add this, but why do you keep using 'or' instead of ||?
This breaks compatibility with some other compilers and is definitely not our code style.
Please read http://dev.minetest.net/Code_style_guidelines
Also, here you're using recursion. How deep is it? This may affect performance, which is especially noticeable on lower-end computers. You need to test the execution time of the old and this new raillike mesh generation to ensure that there is no regression.
And then, diagonal rails are not optional. As pointed out in the comments above, people might not want this, and it's never good to force new features on people, or really, change the way things work when they were already working fine before.
Finally, the code in your patch is rather messy, but I noticed the code it replaces is messy as well. Shouldn't more effort be put in to improving the code itself along with the feature addition?

@ghost
Copy link

ghost commented Mar 17, 2013

...adds realism, but the idea has issues which are quite numerous...the idea of Minetest is that it can run on anything just about, this breaks that ideology...most users don't want this either to my understanding...I have to disagree on the old ideology of don't fix it it's not broken since in my mind everything can be improved

@celeron55
Copy link
Member

I think this is good, with one thing to consider: How hard it is to make carts move diagonally so that they work reasonably with this? And does PilzAdam have any time to update the cart mod? Or will khonkhortisan do that? 8) In any case, merging this without updating the cart mod is not a good idea.

@ghost
Copy link

ghost commented Apr 12, 2013

@celeron55, just curious where's the pull request for mine carts or is there one...such as putting them in the base game? I retract my statement on performance not sure what I was thinking there, think I was sick in bed. I can't see it being too difficult, but the question still remains as to whether the users want this?

@Uberi
Copy link
Contributor

Uberi commented Apr 12, 2013

DJSKIP18, I don't believe there is such a pull request. Rails don't really serve any functional purpose in the main game, besides maybe making bubbles of air underwater to build in.

@khonkhortisan
Copy link
Contributor Author

I changed the and/or to &&/||.
I can see this would be better received if carts also worked diagonally. Right now, carts are very much locked to axes.
Btw, minecraft has diagonal carts but non-diagonal rails, and minetest has non-diagonal carts and (could have) diagonal rails. So that means that it's possible to have both.
I went 114 lines over the 200-line function limit.

@MirceaKitsune
Copy link
Contributor

Was this tested with the carts mod as well, and can carts drive diagonally? Also, can diagonal rails go up / down on nodes like straight rails? If all that works and there are no problems, I support this. Though I am suspicious on how anything diagonal can be achieved in a world of voxel blocks.

@RealBadAngel
Copy link
Contributor

whats the current state of this pull?

@proller
Copy link
Contributor

proller commented Aug 27, 2013

please rebase

@khonkhortisan
Copy link
Contributor Author

I have rebased, and I would like to re-clarify.
This pull request adds code to minetest to make rails visually connect diagonally. They still physically connect the same way.
The child pull request for minetest_game adds two extra images to default:rail.
There is a third pull request to copy default:rail tiles to carts because carts overrides minetest_game. It also adds diagonal versions powered and brake rail images.
Carts move exactly the same as before this pull request. (Carts may or may not be made to move diagonally in the future)
Texture packs may wish to recreate the two extra images in their own style.

Minecraft has diagonal-moving carts on non-diagonal rail. Minetest has non-diagonal-moving carts on diagonal rail. Shouldn't be too hard to have both.

proller pushed a commit to freeminer/freeminer that referenced this pull request Nov 27, 2013
@celeron55
Copy link
Member

Can this be made compatible with games that don't provide the extra rail textures? Currentlly it looks like this:

@celeron55
Copy link
Member

Closing all PRs older than one year. Rebase and re-issue if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

@ Client / Audiovisuals Rebase needed The PR needs to be rebased by its author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants