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

Basic stone walls, using NDT_CONNECTED. #796

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
@sofar
Copy link
Member

commented Jan 21, 2016

These basic connected wall nodes automatically connect
to neigboring stone blocks, other wall blocks and anything
that's "cracky". The do not connect to wood (fences will do
that).

The walls are generated using a new walls.register() API.
Documentation on the API is included in game_api.txt.

This change requires minetest/minetest#3503.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 21, 2016

This was part of the PoC testing code, but these nodes are ready to go and very much usable. As stated, this depends on the connected nodeboxes PR in minetest to be merged.

Once that patch is merged, I will submit a second commit to convert the existing fences to this new nodebox type (the conversion is seamless). However, There is still a fence API PR that is ~~~still pending~~~ as well, that should be merged prior.

@kilbith

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2016

👎 for registering a shitload of walls (11). This is not an obese game like Dreambuilder !

Just stick with the cobble walls and mossy cobble walls.

@kilbith

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2016

Also I find the middle part ridiculously thin and inconsistent compared to the posts. Can you thickened it by 2 pixels and leveled it up by 1 pixel ? Like that :

screenshot_20160121_112501

@sfan5 sfan5 added the enhancement label Jan 21, 2016

@sfan5

This comment has been minimized.

Copy link
Member

commented Jan 21, 2016

👍

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 21, 2016

@kilbith agreed it was a bit over the top with 12 new wall types :)

How about I leave all the cobble walls in (normal, mossy and desert cobble - 3 total) ?

I'm open to adjusting the model - that's what this PR is for. Making the model a bit beefier is certainly a good suggestion. Anyone else?

@kilbith

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2016

How about I leave all the cobble walls in (normal, mossy and desert cobble - 3 total) ?

Perfect.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 21, 2016

Also, I think we can make the model a bit more interesting, and give it the "rustic" appearance.

Here's a slightly thicker wall (6px thick as you requested), but with bevelled tops:

bevelled tops

@kilbith

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2016

Hell, not that 👎 Keep it simple, please.

Detailed nodebox models are also unoptimized (lot of hidden faces/vertices), causing troubles on low-end hardware when these are massively placed.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 21, 2016

You either are cheering as the greatest fan, or vehemently oppose.

How about we let some more folks chime in, first? Unoptimized node boxes seems like an inconsequential issue to me at this point.

@kilbith

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2016

You either are cheering as the greatest fan, or vehemently oppose.

Because I exactly know what this game needs or not, my experience talks in my favor about what thing will succeed or fail. Any move distancing the blocky design and the KISS principle is unwelcome, visually and performatively.

@tenplus1

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2016

+1 for simple walls, I can already feel the lag on the bevelled look.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 21, 2016

@tenplus1 I'm assuming you're serious, so, I will consider it seriously as well.

@red-001

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2016

I prefer the thinner walls, they would look even better if they where higher.

@MoNTE48

This comment has been minimized.

Copy link

commented Jan 21, 2016

👍 +1 for simple walls, too

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2016

While I'm totally not convinced that shaving off the nodeboxes is going to make a drastic framerate difference, I have pushed a version that's as dull as could be:

whatever

If this has people's preference, I can live with that, and I'd rather see these go in than stay out of minetest_game.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2016

BTW I do prefer the thicker look myself.

@rubenwardy

This comment has been minimized.

Copy link
Member

commented Jan 22, 2016

This adds quite nice decoration to things like cottages etc. However, fences already sort of do this, so I'm a little indecisive.

If I were making a subgame, I'd include something like this, because I like they'd look nicer than fenses for certain builds. However, I would probably add more things to tie into it to make it make more sense, like arches and window boxes etc (although they would need to keep torwards the voxel style, and not be homedecor-ish).

I'm leaning towards a 👍 at the moment.

What's the game play difference from this vs fenses? Are they the same apart from looks?

Because I exactly know what this game needs or not, my experience talks in my favor about what thing will succeed or fail.

This strikes me as a little absolute, arrogant and assertive. Just because you have contributed to minetest_game before, and have been active in other ways, doesn't mean that you're automatically that you're the authority on this. Yes, you do have the right to an opinion - but that's not what this sounds like, from the way you phrased it it's very certain and assertive. Particularly the "exactly" part. I guess that this isn't really your intent or what you meant. So I'd appreciate a clarification.

However, saying that, I don't think wood walls makes sense with this draw style. I think that this style should be limited to stone and mossy. Brick walls possibly. I like the bevel on top of the wall, but not on top of the post.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2016

The walls do not have an extended collision box, so they do not block anyone attempting to jump over.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2016

BTW, it is possible to make these nodeboxes extend vertically, and make them unjumpable(*).

unjumpable

The drawback to this method is that the collision box is calculated on the nodeboxes, so you can't make them obstruct a jump and not block the view at the same time. Although it somewhat works out OK.

Plus it seems something that fits in an extended wall mod.

(*)Having them taller still doesn't prevent shift-jumping.

@rubenwardy

This comment has been minimized.

Copy link
Member

commented Jan 22, 2016

I like how you can jump over them, it makes them different from fences.

@rubenwardy

This comment has been minimized.

Copy link
Member

commented Jan 22, 2016

I'd also prefer it if there was only a post every corner, not every node.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2016

a post every corner

@rubenwardy that's a long discussion thread, and I'll attempt to explain why this isn't implemented.

First, consider the logic needed to make this work (another flag, inconsistent visuals)

Second, if a block is placed on top, would you want the post to be drawn? (more options in the nodedef?)

Now, consider that a simple nodebox node with a single fixed nodebox, rotatable, could be trivially maintained and updated by a lua mod to provide that visual aspect for exactly the circumstances where you'd want that to happen.

I've thought this over quite long, and, I realize that I probably can't pleasure anyone in a PR with an approach that attempts to do just that, so, the best way forward is to leave that kind of change to a mod.

Hence, these walls are simple. They can be easily enhanced to do what you want in lua (without making the node count use 15+ nodedefs, like current existing implementations do) with just 1 extra nodedef.

@paramat

This comment has been minimized.

Copy link
Member

commented Jan 22, 2016

Here's a slightly thicker wall (6px thick as you requested), but with bevelled tops:

I prefer the simpler wall design without bevels, more consistent with everything else we have and good for performance.

@C1ffisme

This comment has been minimized.

Copy link

commented Jan 23, 2016

People are going against the bevel top wall...

In my opinion, it looks good, but if it's really causes too much lag than I suppose we shouldn't add it.

@0-afflatus

This comment has been minimized.

Copy link

commented Jan 30, 2016

I don't like this. IMO xwalls mod does it better. A simple uniform thin wall piece is enough, if you want posts you can use normal nodes. It's better than the ugly stone fences, but ultimately I don't actually see the need for this at all. 👐

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2016

@0-afflatus this mod doesn't replace xwalls, however it uses a new game node type that massively allows to reduce node defininitions. In order for mods like xwalls to understand how to benefit from that, there needs to be examples and ready to use code, which this patch adds.

@0-afflatus

This comment has been minimized.

Copy link

commented Jan 30, 2016

@sofar I think I'm not quite understanding your point. Mine is aesthetic and from a building POV, the shape of your walls seems overcomplicated to me. I just think building blocks should be the simplest possible shapes, otherwise they start to dictate building style.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2016

@0-afflatus The name "walls" is perhaps confusing. This isn't a block that usable for the wall of a house, since it leaves obvious gaps. This mod's use of the connected node drawtype is more for decorative brick countryside stone fences. Your use of this block type is entirely voluntary, and you're free to make subgames that exclude these nodes.

@0-afflatus

This comment has been minimized.

Copy link

commented Jan 31, 2016

Sure, that's why I'm trying to avoid voting on most MTG issues. My opinion isn't strictly relevant here.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Feb 10, 2016

rebased.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Feb 17, 2016

Updated the craft recipe to return 6 items, not 1

@C1ffisme

This comment has been minimized.

Copy link

commented Feb 17, 2016

@sofar That makes slightly more sense than the original recipe.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Feb 17, 2016

I was testing this in survival mode, and having to dig 6 cobble stone for 1 wall segment was brutal :)

@C1ffisme

This comment has been minimized.

Copy link

commented Feb 17, 2016

@sofar Yeah, I built things using lots of fences in Minecraft, and getting the wood was a pain. This is probably a good balance. And more realistic.

@paramat

This comment has been minimized.

Copy link
Member

commented Feb 21, 2016

👎 Sorry, beautifully done but i feel unnecessary, we can build walls with nodes, slabs, stairs.

@C1ffisme

This comment has been minimized.

Copy link

commented Feb 21, 2016

@paramat Yes, but they don't really look good. They aren't as nice looking as this pull request.

And I'd like to have an example node to read if I need to make an NDT_CONNECTED node.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Feb 21, 2016

I'll likely submit a PR to replace the default:fence nodes to use this mechanism as well, so that will likely be the canonical example code available.

@C1ffisme

This comment has been minimized.

Copy link

commented Feb 21, 2016

@sofar Probably a good idea. We should completely remove the fence drawtype for good.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2016

I still prefer this to be added, instead of having dozens of other mods implementing something almost the same but perpetuating the non-connected node stuff with 5+ nodedefs, sometimes more.

@sfan5 sfan5 added the enhancement label Feb 29, 2016

@paramat paramat added One approval and removed enhancement labels Mar 2, 2016

@Ekdohibs

This comment has been minimized.

Copy link
Member

commented Mar 3, 2016

👍

@paramat paramat added this to the 0.4.14 milestone Mar 6, 2016

@sofar

This comment has been minimized.

Copy link
Member Author

commented Mar 12, 2016

  • Updated with API changes
@sofar

This comment has been minimized.

Copy link
Member Author

commented Mar 13, 2016

Slight update to the top of the wall. When something is put on top, it grows the last pixel to make it usable as a pillar.

@paramat

This comment has been minimized.

Copy link
Member

commented Mar 13, 2016

How about making the pillars one node high to start with? makes more sense and is simpler. I think this should be considered before merge.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Mar 13, 2016

Kilbith suggested the same as well, so I'm more than welcome to do so. I'll change it.

Basic stone walls, using NDT_CONNECTED.
These basic connected wall nodes automatically connect
to neigboring stone blocks, other wall blocks and anything
that's "cracky". The do not connect to wood (fences will do
that).

The walls are generated using a new walls.register() API.
Documentation on the API is included in game_api.txt.

This change requires minetest/minetest#3503.

Walls are added for all cobble stone materials. They generally
look best and are the natural use cases for these materials.
@paramat

This comment has been minimized.

Copy link
Member

commented Mar 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.