Add deepslate, copper and raw_ores by NO11 #2141

There's the _mcl_mod_origin item field for the APIs registering items with their own namespace discussion that mentioned stairs regeistration. This PR uses registers stairs in a way that is relevant to that discussion.

Is this an issue that should be addressed in this PR or is the discussion in #2088 unresolved?

There's the [`_mcl_mod_origin` item field for the APIs registering items with their own namespace](https://git.minetest.land/MineClone2/MineClone2/issues/2088) discussion that mentioned stairs regeistration. This PR uses registers stairs in a way that is relevant to that discussion. Is this an issue that should be addressed in this PR or is the discussion in #2088 unresolved?

cora commented

2022-04-25 22:31:51 +02:00

It is somewhat unresolved but then i suppose just adding that field would be easy.

NO11 commented

2022-04-25 22:51:42 +02:00

I would remove the override thing in mcl_raw_ores. And you can remove the licences.

👍 2

cora commented

2022-04-26 02:22:18 +02:00

Yeah so I think the only question that remains is if we want the ores to generate.

I see no reason against it.

Yeah so I think the only question that remains is if we want the ores to generate. I see no reason against it.

cora force-pushed deepslate-copper-rawores from 0fe61f02b0 to e2a65a609c

2022-04-26 11:52:36 +02:00

Compare

cora commented

2022-04-26 11:54:11 +02:00

note using git subtree makes rebasing a pain.

cora commented

2022-04-26 11:55:11 +02:00

the only way that worked for me is to completely rebuild the branch manually.

cora commented

2022-04-26 12:06:22 +02:00

Yeah so I think the only question that remains is if we want the ores to generate.

I see no reason against it.

Well there is one reason: It will create unknowns all over the place if people load mcl2 worlds in mlca (in mcl5 the itemstrings appear to be the same surprisingly i would have bet they had stuck this stuff in mcl_core and/or mcl_nether ^^ )

> Yeah so I think the only question that remains is if we want the ores to generate. > > I see no reason against it. Well there is one reason: It will create unknowns all over the place if people load mcl2 worlds in mlca (in mcl5 the itemstrings appear to be the same surprisingly i would have bet they had stuck this stuff in mcl_core and/or mcl_nether ^^ )

kabou commented

2022-04-26 12:32:00 +02:00

note using git subtree makes rebasing a pain.

Is the pre-merge history of these files so very valuable to mineclone2 anyway? I assume that they were all written by one author anyway and the original repository contains the history in case anyone would want to consult it.

> note using git subtree makes rebasing a pain. Is the pre-merge history of these files so very valuable to mineclone2 anyway? I assume that they were all written by one author anyway and the original repository contains the history in case anyone would want to consult it.

kabou commented

2022-04-26 12:45:34 +02:00

Yeah so I think the only question that remains is if we want the ores to generate.

I see no reason against it.

Well there is one reason: It will create unknowns all over the place if people load mcl2 worlds in mlca (in mcl5 the itemstrings appear to be the same surprisingly i would have bet they had stuck this stuff in mcl_core and/or mcl_nether ^^ )

At what point in time are we going to realize that compatibility with mineclone5 and mineclonia becomes realistically untenable(*)? At that point we'll probably be regretting not having made sane choices and instead getting stuck with arbitrariness.

I am strongly urging for a policy to put nodes in the right place, not necessarily where our sister projects may have put or want to put them (unless that is the right place of course.)

For users who want to use world maps from one mineclone in another, we could provide an optional mod that has lbms for all the diverging nodes and items.

(*) I think it is by now plenty obvious that the way things are going, mcl5 has very little review and is all over the place, while mcla has lots of review per line of code added but is not going anywhere.

> > Yeah so I think the only question that remains is if we want the ores to generate. > > > > I see no reason against it. > > Well there is one reason: It will create unknowns all over the place if people load mcl2 worlds in mlca (in mcl5 the itemstrings appear to be the same surprisingly i would have bet they had stuck this stuff in mcl_core and/or mcl_nether ^^ ) At what point in time are we going to realize that compatibility with mineclone5 and mineclonia becomes realistically untenable(*)? At that point we'll probably be regretting not having made sane choices and instead getting stuck with arbitrariness. I am strongly urging for a policy to put nodes in the right place, not necessarily where our sister projects may have put or want to put them (unless that is the right place of course.) For users who want to use world maps from one mineclone in another, we could provide an optional mod that has lbms for all the diverging nodes and items. (*) I think it is by now plenty obvious that the way things are going, mcl5 has very little review and is all over the place, while mcla has lots of review per line of code added but is not going anywhere.

👍 1

cora commented

2022-04-26 15:33:24 +02:00

Is the pre-merge history of these files so very valuable to mineclone2 anyway? I assume that they were all written by one author anyway and the original repository contains the history in case anyone would want to consult it.

Not really. I'm just taking it as a learning opportunity at this point. But I would advise against it for everyone who does not feel secure with rebase --interactive.

At what point in time are we going to realize that compatibility with mineclone5 and mineclonia becomes realistically untenable(*)? At that point we'll probably be regretting not having made sane choices and instead getting stuck with arbitrariness.

I am strongly urging for a policy to put nodes in the right place, not necessarily where our sister projects may have put or want to put them (unless that is the right place of course.)

Well that is the question, what is the right place, isn't it. I think in this particular case all 3 projects are actually on the same page for once: mcl5 already has it in separate mods just like mcla wants it so really no reason for us not to do the same plust as I mentioned earlier it would be good to have these compartmentalized if only to have a setting to disable these at some point.

For users who want to use world maps from one mineclone in another, we could provide an optional mod that has lbms for all the diverging nodes and items.

(*) I think it is by now plenty obvious that the way things are going, mcl5 has very little review and is all over the place, while mcla has lots of review per line of code added but is not going anywhere.

Yes but just because mcl5 doesn't look left and right before merging everything and mcla practically never merges anything (yes these are hyperboles) doesn't mean we should forget they exist.

Ultimately the choices here are often fairly arbitrary. For example the respawn anchor thing, yes it makes some conceptual sense that it would be in mcl_beds but I think erlehmann has a point too that the featureset of mcl_beds is long established and changing that now could lead to problems.

Now if there is a technical reason to do something and it would be hard work or just otherwise be inconvenient I agree but I do not think we should make things incompatible on purpose just because we feel it would fit better or whatever.

I tend to also feel "they" don't have the right to make demands especially with the recent trouble they caused for mcl2 but I also think we should do our due dilligence being responsible about these things - and I think making things harder for the other mcl's just because we want to change some fairly arbitrary identifiers that would be irresponsible.

Now if there's one thing I don't like it's fundamentalism so if you ask me for a dogma about this i have to disappoint but I would suggest we go by this rule of thumb:

Keep the identifiers (particularly itemstrings but also itemgroups and probably other things) if we directly merge things from one of the other mcls
If reasonably possible try to maintain future compatibility but don't bend backwards for it.

"There is no greater evil than men's failure to consult and to consider." -Sophocles (Antigone)

> Is the pre-merge history of these files so very valuable to mineclone2 anyway? I assume that they were all written by one author anyway and the original repository contains the history in case anyone would want to consult it. Not really. I'm just taking it as a learning opportunity at this point. But I would advise against it for everyone who does not feel secure with rebase --interactive. > At what point in time are we going to realize that compatibility with mineclone5 and mineclonia becomes realistically untenable(*)? At that point we'll probably be regretting not having made sane choices and instead getting stuck with arbitrariness. > I am strongly urging for a policy to put nodes in the right place, not necessarily where our sister projects may have put or want to put them (unless that is the right place of course.) Well that is the question, what is the right place, isn't it. I think in this particular case all 3 projects are actually on the same page for once: mcl5 already has it in separate mods just like mcla wants it so really no reason for us not to do the same plust as I mentioned earlier it would be good to have these compartmentalized if only to have a setting to disable these at some point. > For users who want to use world maps from one mineclone in another, we could provide an optional mod that has lbms for all the diverging nodes and items. > > (*) I think it is by now plenty obvious that the way things are going, mcl5 has very little review and is all over the place, while mcla has lots of review per line of code added but is not going anywhere. Yes but just because mcl5 doesn't look left and right before merging everything and mcla practically never merges anything (yes these are hyperboles) doesn't mean we should forget they exist. Ultimately the choices here are often fairly arbitrary. For example the respawn anchor thing, yes it makes *some* conceptual sense that it would be in mcl_beds but I think erlehmann has a point too that the featureset of mcl_beds is long established and changing that now could lead to problems. Now if there is a technical reason to do something and it would be hard work or just otherwise be inconvenient I agree but I do not think we should make things incompatible on purpose just because we feel it would fit better or whatever. I tend to also feel "they" don't have the right to make demands especially with the recent trouble they caused for mcl2 but I also think we should do our due dilligence being responsible about these things - and I think making things harder for the other mcl's just because we want to change some fairly arbitrary identifiers that would be irresponsible. Now if there's one thing I don't like it's fundamentalism so if you ask me for a dogma about this i have to disappoint but I would suggest we go by this rule of thumb: * Keep the identifiers (particularly itemstrings but also itemgroups and probably other things) if we directly merge things from one of the other mcls * If reasonably possible try to maintain future compatibility but don't bend backwards for it. *"There is no greater evil than men's failure to consult and to consider."* -Sophocles (Antigone)

👍 1

erlehmann commented

2022-04-26 15:34:20 +02:00

Yeah so I think the only question that remains is if we want the ores to generate.

I see no reason against it.

Well there is one reason: It will create unknowns all over the place if people load mcl2 worlds in mlca (in mcl5 the itemstrings appear to be the same surprisingly i would have bet they had stuck this stuff in mcl_core and/or mcl_nether ^^ )

Thank you for considering it. In this case I think it is no problem to have these unknown nodes, since the problem can be fixed by installing the deepslate mod from contentdb – as long as nothing is changed in an incompatible way.

> > Yeah so I think the only question that remains is if we want the ores to generate. > > > > I see no reason against it. > > Well there is one reason: It will create unknowns all over the place if people load mcl2 worlds in mlca (in mcl5 the itemstrings appear to be the same surprisingly i would have bet they had stuck this stuff in mcl_core and/or mcl_nether ^^ ) Thank you for considering it. In this case I think it is no problem to have these unknown nodes, since the problem can be fixed by installing the deepslate mod from contentdb – as long as nothing is changed in an incompatible way.

👍 1

cora commented

2022-04-27 03:53:56 +02:00

So uh after that little derailment ... ore registration yes or no ?

I think yes

So uh after that little derailment ... ore registration yes or no ? I think yes

erlehmann commented

2022-04-27 03:59:11 +02:00

Proposal: Make it a config option and default to true.

cora commented

2022-04-27 04:00:26 +02:00

Proposal: Make it a config option and default to true.

already the case (at least for actual ores ... not the stone stuff i think).

> Proposal: Make it a config option and default to true. already the case (at least for actual ores ... not the stone stuff i think).

erlehmann commented

2022-04-27 04:01:02 +02:00

Rationale for making it a config option: In Minecraft, deepslate is not supposed to be generated higher than the original bedrock. And the way it is now, it allows some kind of chunk trailing (presence of deepslate = newer chunk).

erlehmann commented

2022-04-27 04:05:19 +02:00

In fact, making two config options for deepslate generation height minimum and offset upwards or maximum and offset downwards might be the most future proof solution here (if you make two settings for minimum and maximum it is impossible to validate them in the settings dialog).

cora commented

2022-04-27 11:54:04 +02:00

And the way it is now, it allows some kind of chunk trailing (presence of deepslate = newer chunk).

To be fair there's a huge number of things that make this possible - uncarved pumpkins to name just one ^^

But since deepslate is all over the place maybe you're right

> And the way it is now, it allows some kind of chunk trailing (presence of deepslate = newer chunk). To be fair there's a huge number of things that make this possible - uncarved pumpkins to name just one ^^ But since deepslate is all over the place maybe you're right

kneekoo commented

2022-04-27 14:58:42 +02:00

Natural generation
Deepslate makes up the majority of the solid blocks generated below Y=0 in the Overworld. Stone is gradually replaced from Y=8 to Y=0 until it is completely replaced by deepslate.

Deepslate also generates in the deep dark biome and in ancient cities.‌[upcoming: JE 1.19]

> [Natural generation](https://minecraft.fandom.com/wiki/Deepslate#Natural_generation) > Deepslate makes up the majority of the solid blocks generated below Y=0 in the Overworld. Stone is gradually replaced from Y=8 to Y=0 until it is completely replaced by deepslate. > > Deepslate also generates in the deep dark biome and in ancient cities.‌[upcoming: JE 1.19]

kabou commented

2022-04-27 15:40:40 +02:00

As I wrote elsewhere, I am a fan of configuration options for everything, but for now I think we can suffice with deepslate generating in the lowest 8 layers of the world, asa kneekoo pointed out. When generating larger worlds gets implemented, deepslate can be the default stone for the newer depths.

Thinking about it, if we make overworld height and depth configurable, and also find a way to express deepslate generation parameters in a configurable terms(*), then "old world" and "new world" become simple sets of configuration defaults for defined sets of configuration variables. This would allow creation of highly custom worlds.

(*) maybe "deepslate_min_y" and "deepslate_cutoff_length"?

As I wrote elsewhere, I am a fan of configuration options for everything, but for now I think we can suffice with deepslate generating in the lowest 8 layers of the world, asa kneekoo pointed out. When generating larger worlds gets implemented, deepslate can be the default stone for the newer depths. Thinking about it, if we make overworld height and depth configurable, and also find a way to express deepslate generation parameters in a configurable terms(*), then "old world" and "new world" become simple sets of configuration defaults for defined sets of configuration variables. This would allow creation of highly custom worlds. (*) maybe "deepslate_min_y" and "deepslate_cutoff_length"?

cora commented

2022-04-27 15:49:51 +02:00

What if those settings are changed after world creation... This isn't as easy as it

kabou commented

2022-04-27 16:07:47 +02:00

What if those settings are changed after world creation... This isn't as easy as it

Some settings can be immutable after initial world generation, others can be in-game configurable. If users change the immutable options with an editor and their world breaks.. they get to keep the parts.

> What if those settings are changed after world creation... This isn't as easy as it Some settings can be immutable after initial world generation, others can be in-game configurable. If users change the immutable options with an editor and their world breaks.. they get to keep the parts.

erlehmann commented

2022-04-27 16:12:32 +02:00

Some settings can be immutable after initial world generation, others can be in-game configurable

Do you have some example mod that implements stuff like this?

> Some settings can be immutable after initial world generation, others can be in-game configurable Do you have some example mod that implements stuff like this?

cora commented

2022-04-27 16:36:59 +02:00

I think I'll just remove the ores for now else you guys are going to get lost in details and this won't go anywhere

kabou referenced this pull request

2022-04-27 17:11:00 +02:00

`_mcl_mod_origin` item field for the APIs registering items with their own namespace #2088

kabou commented

2022-04-27 17:14:32 +02:00

Some settings can be immutable after initial world generation, others can be in-game configurable

Do you have some example mod that implements stuff like this?

Hopefully soon. I have already outlined some ideas here and here.

> > Some settings can be immutable after initial world generation, others can be in-game configurable > > Do you have some example mod that implements stuff like this? Hopefully soon. I have already outlined some ideas [here](https://git.minetest.land/MineClone2/MineClone2/issues/2131#issuecomment-36917) and [here](https://git.minetest.land/MineClone2/MineClone2/issues/2131#issuecomment-36950).

kabou commented

2022-04-27 17:50:15 +02:00

#2147

cora force-pushed deepslate-copper-rawores from e2a65a609c to ef5759624e

2022-04-28 00:27:04 +02:00

Compare

cora commented

2022-04-28 00:28:20 +02:00

Commented out the ore registration. Now we can hopefully focus on the main point of this pr.

kabou commented

2022-04-28 12:00:00 +02:00

Rationale for making it a config option: In Minecraft, deepslate is not supposed to be generated higher than the original bedrock.

It is though. https://minecraft.fandom.com/wiki/Deepslate#Natural_generation :

Natural generation
Deepslate makes up the majority of the solid blocks generated below Y=0 in the Overworld. Stone is gradually replaced from Y=8 to Y=0 until it is completely replaced by deepslate.

In fact, making two config options for deepslate generation height minimum and offset upwards or maximum and offset downwards might be the most future proof solution here (if you make two settings for minimum and maximum it is impossible to validate them in the settings dialog).

We should also add a deepslate_cutoff parameter to control the above quoted wiki behavior.

> Rationale for making it a config option: In Minecraft, deepslate is not supposed to be generated higher than the original bedrock. It is though. https://minecraft.fandom.com/wiki/Deepslate#Natural_generation : ``` Natural generation Deepslate makes up the majority of the solid blocks generated below Y=0 in the Overworld. Stone is gradually replaced from Y=8 to Y=0 until it is completely replaced by deepslate. ``` > In fact, making two config options for deepslate generation height minimum and offset upwards or maximum and offset downwards might be the most future proof solution here (if you make two settings for minimum and maximum it is impossible to validate them in the settings dialog). We should also add a `deepslate_cutoff` parameter to control the above quoted wiki behavior.

cora commented

2022-04-28 12:07:49 +02:00

This is off topic now. This PR is about adding nodes. I removed the ore spawning.

kabou commented

2022-04-28 13:19:54 +02:00

This is off topic now. This PR is about adding nodes. I removed the ore spawning.

Ah, I wondered where the posts that I replied to went after I posted..

> This is off topic now. This PR is about adding nodes. I removed the ore spawning. Ah, I wondered where the posts that I replied to went after I posted..

cora commented

2022-04-28 17:57:32 +02:00

Uuuh I didn't delete anything if that's what you mean I'll just not waste any more time with pointless discussions its been a bit much recently

kabou commented

2022-04-28 18:51:52 +02:00

Found the posts again, don't know what happened there. Yeah the thread got kind of long winded.

cora force-pushed deepslate-copper-rawores from ef5759624e to c941a17bd6

2022-05-01 03:16:47 +02:00

Compare

cora commented

2022-05-01 03:17:29 +02:00

rebased .. this time without subtrees - it's more trouble than it's worth.

👍 1

AFCMS requested changes 2022-05-02 12:16:46 +02:00

AFCMS left a comment

Looks good in general

Some issues with codestyle

I personally don't like the textures for raw ores (pixelperfection 1.18 ones are better IMO)

Looks good in general Some issues with codestyle I personally don't like the textures for raw ores (pixelperfection 1.18 ones are better IMO)

mods/ITEMS/mcl_copper/README.md Outdated

					
				@ -0,0 +1,10 @@

				# MineClone2 Copper

				### by NO11

AFCMS commented

2022-05-02 12:08:08 +02:00

README shoudl be fixed (remove shields, "You need the mineclone2 subgame...")

screenshot should be added as screenshot.png, not beeing linked to the cdb page

README shoudl be fixed (remove shields, "You need the mineclone2 subgame...") screenshot should be added as screenshot.png, not beeing linked to the cdb page

mods/ITEMS/mcl_copper/crafting.lua Outdated

					
				@ -0,0 +5,4 @@

						{ "mcl_copper:raw_copper", "mcl_copper:raw_copper", "mcl_copper:raw_copper" },

						{ "mcl_copper:raw_copper", "mcl_copper:raw_copper", "mcl_copper:raw_copper" },

					}

				})

AFCMS commented

2022-05-02 12:08:25 +02:00

spaces between craft definitions?

cora commented

2022-05-03 21:46:52 +02:00

care to elaborate ? Where is this found in contributing.md? Also what do you mean? Do you want extra spaces? Do you want some of them removed?

AFCMS commented

2022-05-04 10:10:07 +02:00

minetest.register_craft(....)
minetest.register_craft(....)

=>

minetest.register_craft(....)

minetest.register_craft(....)

(not very important btw)

```lua minetest.register_craft(....) minetest.register_craft(....) ``` => ```lua minetest.register_craft(....) minetest.register_craft(....) ``` (not very important btw)

kabou commented

2022-05-04 10:24:57 +02:00

@AFCMS I think it was already agreed that you can make these changes and commit them to the PR branch yourself if you want. That would make the review process a little easier because then cora doesn't need to bother with those code formatting issues and can focus on the way the code actually works.

👍 1

mods/ITEMS/mcl_deepslate/init.lua Outdated

					
				@ -0,0 +12,4 @@

					"ExtremeHillsM", "ExtremeHillsM_ocean", "ExtremeHillsM_deep_ocean", "ExtremeHillsM_underground",

				}

				if minetest.get_modpath("mcl_item_id") then

AFCMS commented

2022-05-02 12:14:13 +02:00

must be removed

kabou commented

2022-05-03 20:28:15 +02:00

So I tested this PR a bit and noticed that while I can smelt regular ores in a furnace, it doesn't work with the deepslate ores.

Edit: indeed this should be possible: https://minecraft.fandom.com/wiki/Iron_Ore#Smelting_ingredient

So I tested this PR a bit and noticed that while I can smelt regular ores in a furnace, it doesn't work with the deepslate ores. Edit: indeed this should be possible: https://minecraft.fandom.com/wiki/Iron_Ore#Smelting_ingredient

👍 1

cora commented

2022-05-03 22:25:28 +02:00

So I tested this PR a bit and noticed that while I can smelt regular ores in a furnace, it doesn't work with the deepslate ores.

Edit: indeed this should be possible: https://minecraft.fandom.com/wiki/Iron_Ore#Smelting_ingredient

Well since they are only obtainable via /give or creative rn i would say this can be fixed later - although its prob not too hard to do.

But this will only be a problem in the future when these actually spawn - I do not see that happening any time soon

> So I tested this PR a bit and noticed that while I can smelt regular ores in a furnace, it doesn't work with the deepslate ores. > > Edit: indeed this should be possible: https://minecraft.fandom.com/wiki/Iron_Ore#Smelting_ingredient Well since they are only obtainable via /give or creative rn i would say this can be fixed later - although its prob not too hard to do. But this will only be a problem in the future when these actually spawn - I do not see that happening any time soon

kabou commented

2022-05-03 22:25:40 +02:00

Fixes for the absence of deepslate ore smelting pushed to the branch. Also slipped in some trivial whitespace fixes.

AFCMS force-pushed deepslate-copper-rawores from 54453e0475 to 20ebb7066a

2022-05-04 11:12:23 +02:00

Compare

AFCMS commented

2022-05-04 11:12:52 +02:00

omg
I managed to do a force push xD

omg I managed to do a force push xD

AFCMS approved these changes 2022-05-04 11:14:18 +02:00

AFCMS left a comment

code style issues fixed
readme fixed
passes luacheck

- [x] code style issues fixed - [x] readme fixed - [x] passes luacheck

AFCMS commented

2022-05-04 11:15:46 +02:00

I would like to know opinions about the raw ores textures:

Shall we switch to PP 1.18 ones?

(👍 👎)

I would like to know opinions about the raw ores textures: Shall we switch to PP 1.18 ones? (:+1: :-1:)

👍 4

AFCMS added the

nodes

Minecraft >= 1.13

labels 2022-05-04 11:19:28 +02:00

kabou commented

2022-05-04 11:58:59 +02:00

Can you post screenshots of both for visual comparison?

AFCMS commented

2022-05-04 12:42:50 +02:00

PP 1.18 textures:

Raw copper block:

Raw iron block:

Raw gold block:

Raw copper:

Raw iron:

Raw gold:

PP 1.18 textures: Raw copper block: ![image](/attachments/170ac136-dc50-4275-b02a-a260fba855f0) Raw iron block: ![image](/attachments/a7ba028e-f632-46ba-bbae-e91c67abfc78) Raw gold block: ![image](/attachments/483a313a-459a-467e-a1af-2b0170c529f4) Raw copper: ![image](/attachments/585d8db6-cb9a-4ffd-ae4a-8a86fc8a4012) Raw iron: ![image](/attachments/47b5ec05-d129-4295-903a-6ecbc08c9ddf) Raw gold: ![image](/attachments/22d9b119-e819-4796-b759-d16018ed4abf)

image.png

3.6 KiB

image.png

3.1 KiB

image.png

4.8 KiB

image.png

3.0 KiB

image.png

3.1 KiB

image.png

3.0 KiB

image.png

3.0 KiB

AFCMS commented

2022-05-04 12:47:35 +02:00

Actual textures:

Raw copper block:

Raw iron block:

Raw gold block:

Raw copper:

Raw iron:

Raw gold:

Actual textures: Raw copper block: ![image](/attachments/12f2e467-d536-4e4f-97da-1220cc2a736c) Raw iron block: ![image](/attachments/880a8895-d529-4ae1-98e2-0fb1115a1deb) Raw gold block: ![image](/attachments/0cef1e9c-421b-4aab-b4a4-618b18ea084b) Raw copper: ![image](/attachments/a4121e66-be3c-4331-b864-d70a68f02091) Raw iron: ![image](/attachments/ce86723a-6ae9-4c98-9fab-46b6f964c6c7) Raw gold: ![image](/attachments/aec12fc7-0f72-49c8-8ffe-dfb41debb2c0)

image.png

3.3 KiB

image.png

3.5 KiB

image.png

3.6 KiB

image.png

3.4 KiB

image.png

3.6 KiB

image.png

3.5 KiB

kabou commented

2022-05-04 14:15:54 +02:00

Thanks, and very illustrating.

erlehmann commented

2022-05-04 16:03:00 +02:00

I wonder: How many of your fixes can be or are upstreamed?

cora commented

2022-05-04 16:21:53 +02:00

@erlehmann a lot of these are somewhat mcl2 integration specific as for the other ones let's ask @NO11 if they are even going to work on the standalone mods when they are merged here (it is already in mcl5).

I mean making a PR that noone wants just for due bureaucracy reasons seems a bit much don't you think?

@erlehmann a lot of these are somewhat mcl2 integration specific as for the other ones let's ask @NO11 if they are even going to work on the standalone mods when they are merged here (it is already in mcl5). I mean making a PR that noone wants just for due bureaucracy reasons seems a bit much don't you think?

erlehmann commented

2022-05-04 16:26:40 +02:00

I mean making a PR that noone wants just for due bureaucracy reasons seems a bit much don't you think?

What exactly makes you think that upstreaming is just silly busywork?

> I mean making a PR that noone wants just for due bureaucracy reasons seems a bit much don't you think? What exactly makes you think that upstreaming is just silly busywork?

cora commented

2022-05-04 16:58:35 +02:00

I mean making a PR that noone wants just for due bureaucracy reasons seems a bit much don't you think?

What exactly makes you think that upstreaming is just silly busywork?

Because I expect the "upstream" to be unmaintained once this is merged (in fact it might be somewhat unmaintained now) but again let's ask @NO11.

I can make a similar issue there as I made in the blackstone one if it makes you happy - that is still unanswered however: https://github.com/debian044/mcl_blackstone/issues

Let me ask a couple counter questions:

What good does "upstreaming" do if upstream does not exist anymore for all intents and purposes?

How is it not silly busywork if nothing will happen with such a PR ?

How is it bad to ask first before putting in the work ?

> > I mean making a PR that noone wants just for due bureaucracy reasons seems a bit much don't you think? > > What exactly makes you think that upstreaming is just silly busywork? Because I expect the "upstream" to be unmaintained once this is merged (in fact it might be somewhat unmaintained now) but again let's ask @NO11. I can make a similar issue there as I made in the blackstone one if it makes you happy - that is still unanswered however: https://github.com/debian044/mcl_blackstone/issues Let me ask a couple counter questions: What good does "upstreaming" do if upstream does not exist anymore for all intents and purposes? How is it not silly busywork if nothing will happen with such a PR ? How is it bad to ask first before putting in the work ?

kabou referenced this pull request

2022-05-04 17:10:15 +02:00

Add "cobble" group. #2174

kabou reviewed 2022-05-04 17:14:52 +02:00

mods/ITEMS/mcl_deepslate/init.lua Outdated

					
				@ -0,0 +441,4 @@

					},

				})

				minetest.register_craft({

kabou commented

2022-05-04 17:14:52 +02:00

#2174 promises to end the unnecessary duplication of "cobble" crafts.

👍 1

erlehmann commented

2022-05-04 18:07:58 +02:00

Because I expect the "upstream" to be unmaintained once this is merged

Well it is on ContentDB and works fine with Mineclonia, so I do not expect it to go away anytime soon.

How is it bad to ask first before putting in the work ?

In my experience that is something that sounds reasonable, but is less successful than trying to upstream a single patch to see how it goes and then doing more if that is appreciated.

> Because I expect the "upstream" to be unmaintained once this is merged Well it is on ContentDB and works fine with Mineclonia, so I do not expect it to go away anytime soon. > How is it bad to ask first before putting in the work ? In my experience that is something that sounds reasonable, but is less successful than trying to upstream a single patch to see how it goes and then doing more if that is appreciated.

cora commented

2022-05-04 18:50:13 +02:00

You mean people do not respond to questions and/or issues but would totally merge an unsolicited PR?

Sorry but that sounds rather far fetched.

To me it seems this is about fulfilling some bureaucratic ideal.

But ultimately none of this is secret and noone keeps you from making such a PR if it is so important to you.

You mean people do not respond to questions and/or issues but would totally merge an unsolicited PR? Sorry but that sounds rather far fetched. To me it seems this is about fulfilling some bureaucratic ideal. But ultimately none of this is secret and noone keeps you from making such a PR if it is so important to you.

👍 1

kabou commented

2022-05-04 19:10:08 +02:00

https://irc.minetest.net/minetest/2022-05-04#i_5969404

https://irc.minetest.net/minetest/2022-05-04#i_5969419

https://irc.minetest.net/minetest/2022-05-04#i_5969404 https://irc.minetest.net/minetest/2022-05-04#i_5969419

cora commented

2022-05-04 20:11:49 +02:00

My impression was that debiankaios and debian044 are different people but I have no idea really.

cora commented

2022-05-04 20:12:31 +02:00

Also stop snitching @erlehmann

EDIT: seriously, it would be nice to get a warning from you when you do this stuff - this is probably not going to lead to as much trouble as the copyright thing but I do not understand why you keep doing these things.

Have I given you the impression I don't listen to you so that you need to force everything?

Also stop snitching @erlehmann EDIT: seriously, it would be nice to get a warning from you when you do this stuff - this is probably not going to lead to as much trouble as the copyright thing but I do not understand why you keep doing these things. Have I given you the impression I don't listen to you so that you need to force everything?

cora commented

2022-05-04 20:26:07 +02:00

I also find it fascinating that in that chat log none of you even asked the one question this is about if they are indeed the same people lol.

kabou commented

2022-05-04 21:25:11 +02:00

@cora it was intended as "there's your answer". I am not sure indeed that debiankaios is debian044, that is a presumption that I made. I was under the impression that they are the same person, maybe I was wrong, maybe not. In chat debiankaios did not react until I after I had left the chat. You can see that at 16:34 I rejoined and he had just left.

Let's leave the off topic stuff here and get on with the PR.

[edit indeed they are not the same person: this]

@cora it was intended as "there's your answer". I am not sure indeed that debiankaios is debian044, that is a presumption that I made. I was under the impression that they are the same person, maybe I was wrong, maybe not. In chat debiankaios did not react until I after I had left the chat. You can see that at 16:34 I rejoined and he had just left. Let's leave the off topic stuff here and get on with the PR. [edit indeed they are not the same person: [this](https://forum.minetest.net/viewtopic.php?p=397642&sid=c8d7ea628603f5caf5f76181318fadac#p397642)]

NO11 commented

2022-05-04 21:39:02 +02:00

Sorry for my inactivity but I'm just too lazy to read through all of this. What is the problem, what do I have to do?

cora commented

2022-05-04 21:58:37 +02:00

Sorry for my inactivity but I'm just too lazy to read through all of this. What is the problem, what do I have to do?

Are you actively maintaining the mod on github, do you want a PR with (some of) the fixes here ?

> Sorry for my inactivity but I'm just too lazy to read through all of this. What is the problem, what do I have to do? Are you actively maintaining the mod on github, do you want a PR with (some of) the fixes here ?

kabou commented

2022-05-04 23:39:15 +02:00

I have prepared the following commit on this branch, but not pushed it yet because it depends on changes proposed in the "cobble group" PR (#2174).

commit 89ecd72b31e5ac9a9a216300b1f6cab754459eb3

    Add cobbled deepslate to cobble group.
    
    By adding cobbled deepslate to the group "cobble", it automatically
    inherits  all crafting recipes and tool repair capabilities that apply
    to that group.
    
    * Add `cobble=1` to cobbled deepslate node definition groups.  This
      requires a little refactoring of the deepslate variants registration
      function.
    * Remove stone tools, furnace and brewing stand crafting recipes.

I have prepared the following commit on this branch, but not pushed it yet because it depends on changes proposed in the "cobble group" PR (#2174). ``` commit 89ecd72b31e5ac9a9a216300b1f6cab754459eb3 Add cobbled deepslate to cobble group. By adding cobbled deepslate to the group "cobble", it automatically inherits all crafting recipes and tool repair capabilities that apply to that group. * Add `cobble=1` to cobbled deepslate node definition groups. This requires a little refactoring of the deepslate variants registration function. * Remove stone tools, furnace and brewing stand crafting recipes. ```

kabou commented

2022-05-04 23:41:17 +02:00

I have prepared the following commit on this branch, but not pushed it yet because it depends on changes proposed in the "bone meal API + mcl_dye fixes" PR (#2158).

commit 40f822d46ab834694ab661095fc8c2b30a110850

    Make deepslate with lapis drop the correct item.
    
    * Replace "mcl_dye:blue" with "mcl_core:lapis" as the drop and smelting
      result of deepslate with lapis.

I have prepared the following commit on this branch, but not pushed it yet because it depends on changes proposed in the "bone meal API + mcl_dye fixes" PR (#2158). ``` commit 40f822d46ab834694ab661095fc8c2b30a110850 Make deepslate with lapis drop the correct item. * Replace "mcl_dye:blue" with "mcl_core:lapis" as the drop and smelting result of deepslate with lapis. ```

cora force-pushed deepslate-copper-rawores from 89ecd72b31 to 84a31b1803

2022-05-05 01:35:19 +02:00

Compare

cora force-pushed deepslate-copper-rawores from 84a31b1803 to 1266396e1d

2022-05-05 01:46:16 +02:00

Compare

cora commented

2022-05-05 02:40:11 +02:00

did a final test of placing the nodes, crafting some stuff with both cobbles and waited a while for some copper to oxidize - all worked out .

cora merged commit 6674684998 into master

2022-05-05 02:40:33 +02:00

cora referenced this issue from a commit

2022-05-05 02:40:33 +02:00

Merge pull request 'Add deepslate, copper and raw_ores by NO11' (#2141) from deepslate-copper-rawores into master

cora deleted branch deepslate-copper-rawores

2022-05-05 02:40:36 +02:00

cora commented

2022-05-05 02:41:39 +02:00

good job guys <3

kabou referenced this pull request from MineClone5/MineClone5

2022-05-05 12:10:15 +02:00

Fire ignited on soul sand/soil isn't blue #303

kabou commented

2022-05-05 13:13:48 +02:00

Continued from here

I find it a little unfortunate that recently so much energy was spent on unrelated and overcharged discussions about code formatting and the gravity of rules. Meanwhile, the real pressing issues, where to place things and under what name, did not receive the ample consideration it deserves.

Personally I had rather seen deepslate, raw ores and copper integrated with mcl_core. These mods comprise mostly of registrations of nodes to be used in mapgen. Although this PR has now been merged, we still have a window of opportunity to move things around before a release is made and the node names become literally fixed in stone in user worlds.

From a purely technical point of view, I see few issues:

Raw ores obviously has no dependencies.
Copper only has a real dependency on mcl_stairs, the mcl_sounds is also an mcl_core dependency and not a real dependency as it is only used in node definition and no calls are made during initialization. If the stairs registration is moved out into mcl_stairs, that dependency is gone.
Deepslate lists many dependencies but upon closer inspection almost all of those are unnecessary:
- mcl_walls, mcl_stairs: we can put the registrations in those mods,
- mcl_worlds is already a dependency of mcl_core,
- mcl_raw_ores, mcl_copper: node drops are not real dependencies, void if these were also put in mcl_core,
- mcl_dye (for the lapis ore drop), not a real dependency, also moot once the dyes are fixed and lapis is on mcl_core proper,
- mcl_brewing, mcl_furnaces, mcl_farming, crafting recipes do not create real dependencies, and the crafting recipes were removed anyway,
- mobs_mc (for silverfishes in infested blocks), not a real dependency and also something that mcl_core has to do already,
- mcl_util, mcl_sounds, screwdriver, walkover: part of node definitions, not called during initialization, so no real dependencies,
- mcl_mobitems: couldn't actually find any use.

It is understandable that an external mod lists dependencies in the most abundant fashion, because it has no knowledge if the functionality is present at all. But we only care about the order of initialization at startup time and that decides the real dependencies.

Often an argument is made that we should split up new additions as much as possible so that external mods can safely depend on the presence of functionality. As I have said before, external mcl2 mods should simply depend on the mcl2 version. To facilitate this we chould provide an interface for mods to check the correct version on initialization.

PS: I just had a brief look at mcl_core and it seems to many of its dependencies can also be reconsidered on similar grounds.

[Continued from here](https://git.minetest.land/MineClone5/MineClone5/issues/303#issuecomment-38019) I find it a little unfortunate that recently so much energy was spent on unrelated and overcharged discussions about code formatting and the gravity of rules. Meanwhile, the real pressing issues, where to place things and under what name, did not receive the ample consideration it deserves. Personally I had rather seen deepslate, raw ores and copper integrated with mcl_core. These mods comprise mostly of registrations of nodes to be used in mapgen. Although this PR has now been merged, we still have a window of opportunity to move things around before a release is made and the node names become literally fixed in stone in user worlds. From a purely technical point of view, I see few issues: * [Raw ores](https://git.minetest.land/MineClone2/MineClone2/src/branch/master/mods/ITEMS/mcl_raw_ores/mod.conf) obviously has no dependencies. * [Copper](https://git.minetest.land/MineClone2/MineClone2/src/branch/master/mods/ITEMS/mcl_copper/mod.conf) only has a real dependency on mcl_stairs, the mcl_sounds is also an mcl_core dependency and not a real dependency as it is only used in node definition and no calls are made during initialization. If the stairs registration is moved out into mcl_stairs, that dependency is gone. * [Deepslate](https://git.minetest.land/MineClone2/MineClone2/src/branch/master/mods/ITEMS/mcl_deepslate/mod.conf) lists many dependencies but upon closer inspection almost all of those are unnecessary: * mcl_walls, mcl_stairs: we can put the registrations in those mods, * mcl_worlds is already a dependency of mcl_core, * mcl_raw_ores, mcl_copper: node drops are not real dependencies, void if these were also put in mcl_core, * mcl_dye (for the lapis ore drop), not a real dependency, also moot once the dyes are fixed and lapis is on mcl_core proper, * mcl_brewing, mcl_furnaces, mcl_farming, crafting recipes do not create real dependencies, and the crafting recipes were removed anyway, * mobs_mc (for silverfishes in infested blocks), not a real dependency and also something that mcl_core has to do already, * mcl_util, mcl_sounds, screwdriver, walkover: part of node definitions, not called during initialization, so no real dependencies, * mcl_mobitems: couldn't actually find any use. It is understandable that an external mod lists dependencies in the most abundant fashion, because it has no knowledge if the functionality is present at all. But we only care about the order of initialization at startup time and that decides the real dependencies. Often an argument is made that we should split up new additions as much as possible so that external mods can safely depend on the presence of functionality. As I have said before, external mcl2 mods should simply depend on the mcl2 version. To facilitate this we chould provide an interface for mods to check the correct version on initialization. PS: I just had a brief look at mcl_core and it seems to many of its dependencies can also be reconsidered on similar grounds.

cora commented

2022-05-05 14:03:28 +02:00

As i said before the dependency system is there fore a reason. Try stuffing everything in mcl_core and you will find you will have much of the same problems only much more complicated to resolve.

Why do some people keep pretending dependencies were made to make their life hard. I just don't get it.

Also apparently you do like these discussions. Why are you rehashing it now?

As i said before the dependency system is there fore a reason. Try stuffing everything in mcl_core and you will find you will have much of the same problems only much more complicated to resolve. Why do some people keep pretending dependencies were made to make their life hard. I just don't get it. Also apparently you do like these discussions. Why are you rehashing it now?

kabou commented

2022-05-05 15:11:41 +02:00

As i said before the dependency system is there fore a reason.

That reason exists in minetest mod ecosystem to organize functional dependencies between mods and provide initialization structuring. For us, only the latter is relevant because the functional dependencies are resolved once, when the mod is integrated in the game.

Try stuffing everything in mcl_core and you will find you will have much of the same problems only much more complicated to resolve.

I don't disagree because that is not what I was saying.

Also apparently you do like these discussions. Why are you rehashing it now?

Because if I don't, then the itemstrings will be cast in stone and it will be nasty to change anything.

So I am not making a point of avoiding mod dependencies, I am trying to make a point about "logical" placement of items, nodes and codes. And, yes, there is some amount of personal taste involved, but that can be said of all sides of the argument.

> As i said before the dependency system is there fore a reason. That reason exists in minetest mod ecosystem to organize functional dependencies between mods and provide initialization structuring. For us, only the latter is relevant because the functional dependencies are resolved once, when the mod is integrated in the game. >Try stuffing everything in mcl_core and you will find you will have much of the same problems only much more complicated to resolve. I don't disagree because that is not what I was saying. > Also apparently you do like these discussions. Why are you rehashing it now? Because if I don't, then the itemstrings will be cast in stone and it will be nasty to change anything. So I am not making a point of avoiding mod dependencies, I am trying to make a point about "logical" placement of items, nodes and codes. And, yes, there is some amount of personal taste involved, but that can be said of all sides of the argument.

cora commented

2022-05-05 16:16:10 +02:00

So I am not making a point of avoiding mod dependencies, I am trying to make a point about "logical" placement of items, nodes and codes. And, yes, there is some amount of personal taste involved, but that can be said of all sides of the argument.

MineClone2/MineClone2#2146 (comment)

> So I am not making a point of avoiding mod dependencies, I am trying to make a point about "logical" placement of items, nodes and codes. And, yes, there is some amount of personal taste involved, but that can be said of all sides of the argument. https://git.minetest.land/MineClone2/MineClone2/pulls/2146#issuecomment-37404

cora commented

2022-05-05 16:18:01 +02:00

No it cant be said for all sides because my "side" is that it's completely arbitrary and if you make up "logical" reasons for it you can easily come up with similarly logical reasons to put it somewhere else.

The reasoning for this stuff should be technical and not (high-level) logical.

This is not part of the ingame world. it's technicaly identifiers.

No it cant be said for all sides because my "side" is that it's completely arbitrary and if you make up "logical" reasons for it you can easily come up with similarly logical reasons to put it somewhere else. The reasoning for this stuff should be technical and not (high-level) logical. This is not part of the ingame world. it's technicaly identifiers.

kabou commented

2022-05-05 23:56:05 +02:00

MineClone2/MineClone2#2146 (comment)

That thread is a good example of how an open discussion, even if it does not initially yield general agreement or paractical results, can eventually bring about interesting insights and left-field solutions: MineClone2/MineClone2#2146 (comment)

This is not part of the ingame world. it's technicaly identifiers.

So glad that we are on the same page here lol.

> https://git.minetest.land/MineClone2/MineClone2/pulls/2146#issuecomment-37404 That thread is a good example of how an open discussion, even if it does not initially yield general agreement or paractical results, can eventually bring about interesting insights and left-field solutions: https://git.minetest.land/MineClone2/MineClone2/pulls/2146#issuecomment-37462 > This is not part of the ingame world. it's technicaly identifiers. So glad that we are on the same page [here](https://git.minetest.land/MineClone2/MineClone2/pulls/2150#issuecomment-37885) lol.

erlehmann commented

2022-05-06 00:03:39 +02:00

Often an argument is made that we should split up new additions as much as possible so that external mods can safely depend on the presence of functionality. As I have said before, external mcl2 mods should simply depend on the mcl2 version. To facilitate this we chould provide an interface for mods to check the correct version on initialization.

I doubt you have a really good understanding of how dependencies work in Minetest …

e.g. mods that games provide could be overridden by extra mods that users activate.

Another thing: ContentDB can show you which games provide mods that your mod needs.

> Often an argument is made that we should split up new additions as much as possible so that external mods can safely depend on the presence of functionality. As I have said before, external mcl2 mods should simply depend on the mcl2 version. To facilitate this we chould provide an interface for mods to check the correct version on initialization. I doubt you have a really good understanding of how dependencies work in Minetest … e.g. mods that games provide could be overridden by extra mods that users activate. Another thing: ContentDB can show you which games provide mods that your mod needs.

cora commented

2022-05-06 00:04:18 +02:00

Pretty sure I have told you many times why mcl_fungus is almsot as bad a name as mcl_mushroom (easy to confuse with mcl_mushrooms).

kabou commented

2022-05-06 00:43:03 +02:00

e.g. mods that games provide could be overridden by extra mods that users activate.

Sure they can. BTW I prefer to call it "functionality that games provide". Any general aggregation of mods is a weak model of "a game". But how is that an argument anyway? If the external mods are considerate of initialization ordering and declare dependencies accordingly they will be fine.

Another thing: ContentDB can show you which games provide mods that your mod needs.

So? ContentDB could add versioning. Don't you agree that it would improve the dependency scheme?

> e.g. mods that games provide could be overridden by extra mods that users activate. Sure they can. BTW I prefer to call it "functionality that games provide". Any general aggregation of mods is a weak model of "a game". But how is that an argument anyway? If the external mods are considerate of initialization ordering and declare dependencies accordingly they will be fine. > Another thing: ContentDB can show you which games provide mods that your mod needs. So? ContentDB could add versioning. Don't you agree that it would improve the dependency scheme?

erlehmann commented

2022-05-06 17:54:29 +02:00

So? ContentDB could add versioning.

“Someone could do a thing to fix bugs I introduced.” is often both true and useless.

Don't you agree that it would improve the dependency scheme?

Yes, I don't. Look at all the mods declaring compatibility with any Minetest version.

> So? ContentDB could add versioning. “Someone could do a thing to fix bugs I introduced.” is often both true and useless. > Don't you agree that it would improve the dependency scheme? Yes, I don't. Look at all the mods declaring compatibility with any Minetest version.

Sign in to join this conversation.

No reviewers