Mineclonia
/
Mineclonia
Archived
12
17
Fork 13
Code Issues 105 Pull Requests 19 Projects Releases Wiki Activity

MAPGEN/tsm_railcorridors: Generate pig spawners in mineshafts #127

Merged
erlehmann merged 1 commits from mineshaft-pig-spawners into master 2021-07-23 04:24:18 +02:00
Conversation 8 Commits 1 Files Changed 2 +20 -5
erlehmann commented 2021-07-22 19:35:20 +02:00
Owner
Copy Link
Problem

TRACKING ISSUE: #126

In Minecraft, cave spider spawners in mineshafts are very rarely replaced by pig spawners.

Solution

This patch makes 1 in 1000 spawners in a mineshaft be a pig spawner instead of a cave spider spawner.

Details

The behaviour is not exactly like in Minecraft, because the map generator works differently.

What matters is that there exists a very rare occurence of a naturally generated pig spawner.

Testing Steps
Verify Bug
  1. In Mineclonia commit eddbfb4b5c, generate a world with mapgen v7 and the seed “pigs”.
  2. Enter the newly generated world in creative mode with damage turned off.
  3. Verify that there exists a cave spider spawner at or near coordinates 3,-8,7.
  4. Wait for some cave spiders to spawn.
  5. Verify that there exist cave spider spawners at or near the following coordinates:
    • -75,-7,-73
    • -106,-44,-211
    • -224,-45,-244
Verify Patch
  1. In Mineclonia commit 71cff7051f, generate a world with mapgen v7 and the seed “pigs”.
  2. Enter the newly generated world in creative mode with damage turned off.
  3. Verify that there exists a pig spawner at or near coordinates 3,-8,7.
  4. Wait for some pigs to spawn.
  5. Verify that there exist cave spider spawners at or near the following coordinates:
    • -75,-7,-73
    • -106,-44,-211
    • -224,-45,-244
To do
  • Fill out issue template
  • Write testing instructions
  • Verify testing instructions
##### Problem TRACKING ISSUE: https://git.minetest.land/Mineclonia/Mineclonia/issues/126 In Minecraft, cave spider spawners in mineshafts are very rarely replaced by pig spawners. ##### Solution This patch makes 1 in 1000 spawners in a mineshaft be a pig spawner instead of a cave spider spawner. ##### Details The behaviour is not exactly like in Minecraft, because the map generator works differently. What matters is that there exists a very rare occurence of a naturally generated pig spawner. ##### Testing Steps ###### Verify Bug 1. In Mineclonia commit eddbfb4b5c6c1e15a4314000b6fdc0671d043d0f, generate a world with mapgen v7 and the seed “pigs”. 2. Enter the newly generated world in creative mode with damage turned off. 3. Verify that there exists a cave spider spawner at or near coordinates 3,-8,7. 4. Wait for some cave spiders to spawn. 5. Verify that there exist cave spider spawners at or near the following coordinates: - -75,-7,-73 - -106,-44,-211 - -224,-45,-244 ###### Verify Patch 1. In Mineclonia commit 71cff7051f7889ab85c1d374c79782a60d0ef946, generate a world with mapgen v7 and the seed “pigs”. 2. Enter the newly generated world in creative mode with damage turned off. 3. Verify that there exists a pig spawner at or near coordinates 3,-8,7. 4. Wait for some pigs to spawn. 5. Verify that there exist cave spider spawners at or near the following coordinates: - -75,-7,-73 - -106,-44,-211 - -224,-45,-244 ##### To do - [x] Fill out issue template - [x] Write testing instructions - [x] Verify testing instructions
erlehmann changed title from Generate pig spawners in mineshafts to MAPGEN/tsm_railcorridors: Generate pig spawners in mineshafts 2021-07-22 19:36:43 +02:00
e added the
missing feauture
 label 2021-07-23 03:47:38 +02:00
e requested changes 2021-07-23 04:06:40 +02:00
e left a comment
Member

Code Review

One problem regarding an accidental global definition.

Testing Steps

Verify Bug

  • In Mineclonia commit eddbfb4b5c, generate a world with mapgen v7 and the seed pigs.
  • Enter the newly generated world in creative mode with damage turned off.
  • Verify that there exists a cave spider spawner at or near coordinates 3,-8,7.
  • Wait for some cave spiders to spawn.
  • Verify that there exist cave spider spawners at or near the following coordinates:
    • -75,-7,-73
    • -106,-44,-211
    • -224,-45,-244

Verify Patch

  • In Mineclonia commit e24a49d013, generate a world with mapgen v7 and the seed pigs.
  • Enter the newly generated world in creative mode with damage turned off.
    NB: This message was printed to console.
2021-07-22 21:54:20: WARNING[Emerge-0]: Assignment to undeclared global "pr_spawner" inside a function at .../games/mcla-devel/mods/MAPGEN/tsm_railcorridors/init.lua:127.

See the comment for more details.

  • Verify that there exists a pig spawner at or near coordinates 3,-8,7.
  • Wait for some pigs to spawn.
  • Verify that there exist cave spider spawners at or near the following coordinates:
    • -75,-7,-73
    • -106,-44,-211
    • -224,-45,-244
### Code Review One problem regarding [an accidental global definition](https://git.minetest.land/Mineclonia/Mineclonia/pulls/127/files#issuecomment-25976). ### Testing Steps #### Verify Bug - [x] In Mineclonia commit eddbfb4b5c, generate a world with mapgen v7 and the seed `pigs`. - [x] Enter the newly generated world in creative mode with damage turned off. - [x] Verify that there exists a cave spider spawner at or near coordinates 3,-8,7. - [x] Wait for some cave spiders to spawn. - [x] Verify that there exist cave spider spawners at or near the following coordinates: - [x] -75,-7,-73 - [x] -106,-44,-211 - [x] -224,-45,-244 #### Verify Patch - [x] In Mineclonia commit e24a49d013, generate a world with mapgen v7 and the seed `pigs`. - [x] Enter the newly generated world in creative mode with damage turned off. **NB: This message was printed to console.** ``` 2021-07-22 21:54:20: WARNING[Emerge-0]: Assignment to undeclared global "pr_spawner" inside a function at .../games/mcla-devel/mods/MAPGEN/tsm_railcorridors/init.lua:127. ``` **See the [comment](https://git.minetest.land/Mineclonia/Mineclonia/pulls/127/files#issuecomment-25976) for more details.** - [x] Verify that there exists a pig spawner at or near coordinates 3,-8,7. - [x] Wait for some pigs to spawn. - [x] Verify that there exist cave spider spawners at or near the following coordinates: - [x] -75,-7,-73 - [x] -106,-44,-211 - [x] -224,-45,-244
mods/MAPGEN/tsm_railcorridors/init.lua Outdated
@ -124,2 +124,4 @@
-- Separate randomizer for carts because spawning carts is very timing-dependent
pr_carts = PseudoRandom(seed-654)
-- Spawner type randomiser
pr_spawner = PseudoRandom(seed+345)
e commented 2021-07-23 04:03:54 +02:00
Member

This is being declared globally instead of locally. See L117

This is being declared globally instead of locally. See [L117](https://git.minetest.land/Mineclonia/Mineclonia/src/commit/e24a49d013c15b1fb07389beb3b4d7671888bd82/mods/MAPGEN/tsm_railcorridors/init.lua#L117)
erlehmann commented 2021-07-23 04:14:00 +02:00
Author
Owner

I fixed that by declaring it local like the other pr_ variable (and edited the history via git commit --amend, forgive me). Does it work now?

I fixed that by declaring it local like the other `pr_` variable (and edited the history via `git commit --amend`, forgive me). Does it work now?
e marked this conversation as resolved
e added the
confirmed
 label 2021-07-23 04:07:32 +02:00
erlehmann force-pushed mineshaft-pig-spawners from e24a49d013 to 71cff7051f 2021-07-23 04:12:23 +02:00 Compare
e approved these changes 2021-07-23 04:23:06 +02:00
e left a comment
Member

Previous issue resolved, test results otherwise unchanged.

[Previous issue resolved](https://git.minetest.land/Mineclonia/Mineclonia/issues/127#issuecomment-25980), [test results otherwise unchanged](https://git.minetest.land/Mineclonia/Mineclonia/issues/127#issuecomment-25977).
erlehmann merged commit 45cdad7283 into master 2021-07-23 04:24:18 +02:00
erlehmann deleted branch mineshaft-pig-spawners 2021-07-23 04:24:59 +02:00
erlehmann commented 2021-07-23 16:57:43 +02:00
Author
Owner
Copy Link

I really should not have merged this. One of the spawners was missing for cora on both Waspsaliva and Clamity Minetest – even if it was working for me on Minetest 5.5-dev and e (on Minetest 5.3 or 5.4 I guess, I should have asked here).

I suppose there is something wrong with the tsm_railcorridor generation being non-deterministic – maybe because it is based on the map seed and not on the block seed. I will investigate now.

I really should not have merged this. One of the spawners was missing for cora on both Waspsaliva and Clamity Minetest – even if it was working for me on Minetest 5.5-dev and e (on Minetest 5.3 or 5.4 I guess, I should have asked here). I suppose there is something wrong with the tsm_railcorridor generation being non-deterministic – maybe because it is based on the map seed and not on the block seed. I will investigate now.
LizzyFleckenstein03 commented 2021-07-26 18:42:37 +02:00
Contributor
Copy Link

In Minecraft the cave spider spawners are only replaced if they attempt to generate at the same location as a loot chest. It's a bug.

The fact that they are pig spawners is because spawners are pig spawners by default (which is the case in MCL as well)

tbh if you replicate these bugs you should add Minecraft dupe glitches as well.

In Minecraft the cave spider spawners are only replaced if they attempt to generate at the same location as a loot chest. It's a bug. The fact that they are pig spawners is because spawners are pig spawners by default (which is the case in MCL as well) tbh if you replicate these bugs you should add Minecraft dupe glitches as well.
cora commented 2021-07-26 18:49:11 +02:00
Member
Copy Link

In Minecraft the cave spider spawners are only replaced if they attempt to generate at the same location as a loot chest. It's a bug.

The fact that they are pig spawners is because spawners are pig spawners by default (which is the case in MCL as well)

tbh if you replicate these bugs you should add Minecraft dupe glitches as well.

hey why do i agree with you all the time today ? :D i.e. I wasn't aware this is a bug in mc so yeah i think this shouldnt have been merged – however the impact seems neglibible.

> In Minecraft the cave spider spawners are only replaced if they attempt to generate at the same location as a loot chest. It's a bug. > > The fact that they are pig spawners is because spawners are pig spawners by default (which is the case in MCL as well) > > tbh if you replicate these bugs you should add Minecraft dupe glitches as well. hey why do i agree with you all the time today ? :D i.e. I wasn't aware this is a bug in mc so yeah i think this shouldnt have been merged – however the impact seems neglibible.
LizzyFleckenstein03 commented 2021-07-26 18:51:20 +02:00
Contributor
Copy Link

And no, it's not a feature (like @erlehmann claimed in the issue). I've read the code.

And no, it's not a feature (like @erlehmann claimed in the issue). I've read the code.
erlehmann commented 2021-07-26 19:04:30 +02:00
Author
Owner
Copy Link

@EliasFleckenstein03 ok so if it is not a “feature” why did they not fix it for like 10 years or so?

I think pig spawners are similar to TNT entity duping or redstone quasi connectivity or cake ladders.

One can look at it, think “that does not make sense”, then dive into the history and the bug tracker and realize that Mojang knows, but never fixed it (and in case of quasi connectivity even said they would keep it) and then it does not matter if it looks weird.

In Minecraft the cave spider spawners are only replaced if they attempt to generate at the same location as a loot chest. It's a bug.

I think we do not have “loot chests” (those would be affected by luck?), or do we? Also I think that is only true for the weird thing with five spider spawners, but many other things seem to be able to convert any spawner into a pig spawner, like picking it up and placing it again. I have read a lot about pig spawners, many people found them in different versions of Minecraft, without glitching.

The fact that they are pig spawners is because spawners are pig spawners by default (which is the case in MCL as well)

Ok, that is on a technical level. The level I thought about the issue was “in Minecraft, for many years, you could find a pig spawner in a mineshaft, but it is rare”, i.e. the “it works on Youtube” angle.

tbh if you replicate these bugs you should add Minecraft dupe glitches as well.

I actually agree here – but I also think that stuff that is too controversial should be hidden behind a setting (and not be default).

@EliasFleckenstein03 ok so if it is not a “feature” why did they not fix it for like 10 years or so? I think pig spawners are similar to TNT entity duping or redstone quasi connectivity or cake ladders. One can look at it, think “that does not make sense”, then dive into the history and the bug tracker and realize that Mojang knows, but never fixed it (and in case of quasi connectivity even said they would keep it) and then it does not matter if it looks weird. > In Minecraft the cave spider spawners are only replaced if they attempt to generate at the same location as a loot chest. It's a bug. I think we do not have “loot chests” (those would be affected by luck?), or do we? Also I think that is only true for the weird thing with five spider spawners, but many other things seem to be able to convert any spawner into a pig spawner, like picking it up and placing it again. I have read a lot about pig spawners, many people found them in different versions of Minecraft, without glitching. > The fact that they are pig spawners is because spawners are pig spawners by default (which is the case in MCL as well) Ok, that is on a technical level. The level I thought about the issue was “in Minecraft, for many years, you could find a pig spawner in a mineshaft, but it is rare”, i.e. the “it works on Youtube” angle. > tbh if you replicate these bugs you should add Minecraft dupe glitches as well. I actually agree here – but I also think that stuff that is too controversial should be hidden behind a setting (and not be default).
erlehmann commented 2021-07-26 19:26:04 +02:00
Author
Owner
Copy Link

And no, it's not a feature (like @erlehmann claimed in the issue). I've read the code.

I will not read the code for legal reasons, but what in the code suggests it is not a feature?

I mean, the gaming world is full of such stuff, even only considering Minecraft:

  • TNT duping and carpet duping are known, have been known, and were never fixed.

  • Green robe villagers could not be traded with for a long time. They became Nitwits.

  • You can build on the Nether roof (and mushrooms being generated there, missing in Minecl*).

> And no, it's not a feature (like @erlehmann claimed in the issue). I've read the code. I will not read the code for legal reasons, but what in the code suggests it is not a feature? I mean, the gaming world is full of such stuff, even only considering Minecraft: * TNT duping and carpet duping are known, have been known, and were never fixed. * Green robe villagers could not be traded with for a long time. They became Nitwits. * You can build on the Nether roof (and mushrooms being generated there, missing in Minecl*).
erlehmann commented 2021-07-26 19:35:05 +02:00
Author
Owner
Copy Link

Examples from other games:

  • Quake has strafe jumping, rocket jumping, grenade jumping, plasma sliding …

  • In Space Invaders the speeding up of the game was because it had less to render.

  • In the original Tetris, ppl claimed the T-spin was a bug. It was never fixed though.

Examples from other games: * Quake has strafe jumping, rocket jumping, grenade jumping, plasma sliding … * In Space Invaders the speeding up of the game was because it had less to render. * In the original Tetris, ppl claimed the T-spin was a bug. It was never fixed though.
LizzyFleckenstein03 commented 2021-08-06 11:48:30 +02:00
Contributor
Copy Link

Green robe villagers could not be traded with for a long time. They became Nitwits.

That is not a bug at all.

There is still a difference between "glitches" that are cause by Minecrafts "official" mechanics - a good example for this are cake ladders that are made possible by the properties that the cake was MEANT to have (Not sure if this also goes for Quasi connectivity, but Quasi connectivity has effectively become a feature) and things that are caused by the implementation of the idea (like dupe glitches or glitched pig spawners). MineClone has several dupe glitches that are completely different from the Minecraft ones. It's cool to have glitches that are fun, but putting them into the game on purpose is just not what any good piece of software should do.

We should just not implement any Minecraft bugs (with a few exceptions like quasi connectivity maybe) - rather we should implement the Minecraft mechanics as exact as possible. If this results in bugs that Minecraft has as well, we know we implemented the mechanics in the proper way, and it's a good thing. However some bugs are just specific to a certain implementation. We should not treat Minecraft like a black box, look at what it spits out and try to make something that looks similar. Instead, if we really want to make a good clone that even does things we don't know about we have to implement the mechanics, not the main- and side effects of the mechanics. That does not mean we need to implement everything exactly as it was implemented in Minecraft. We just need to clone the logic rather than the output.

> Green robe villagers could not be traded with for a long time. They became Nitwits. That is not a bug at all. There is still a difference between "glitches" that are cause by Minecrafts "official" mechanics - a good example for this are cake ladders that are made possible by the properties that the cake was MEANT to have (Not sure if this also goes for Quasi connectivity, but Quasi connectivity has effectively become a feature) and things that are caused by the implementation of the idea (like dupe glitches or glitched pig spawners). MineClone has several dupe glitches that are completely different from the Minecraft ones. It's cool to have glitches that are fun, but putting them into the game on purpose is just not what any good piece of software should do. We should just not implement **any** Minecraft bugs (with a few exceptions like quasi connectivity maybe) - rather we should implement the Minecraft mechanics as exact as possible. If this results in bugs that Minecraft has as well, we know we implemented the mechanics in the proper way, and it's a good thing. However some bugs are just specific to a certain implementation. We should not treat Minecraft like a black box, look at what it spits out and try to make something that looks similar. Instead, if we really want to make a good clone _that even does things we don't know about_ we have to implement the mechanics, not the main- and side effects of the mechanics. That does **not** mean we need to implement everything exactly as it was implemented in Minecraft. We just need to clone the logic rather than the output.
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
e
Labels
Clear labels   
blocker

blocks release until resolved

  
bug

   
code quality

   
confirmed

   
critical

crashes, catastrophic bugs

  
discussion

   
high priority

   
incompatibility

incompatible with mcl2

  
incomplete feature

   
invalid

   
low priority

   
missing feauture

   
needs testing

   
packet spam

   
performance

   
project

meta-issues

  
regression

   
translations

   
unconfirmed
  
in review

   
ready for review
No Label
blocker
bug
code quality
confirmed
critical
discussion
high priority
incompatibility
incomplete feature
invalid
low priority
missing feauture
needs testing
packet spam
performance
project
regression
translations
unconfirmed
in review
ready for review
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Mineclonia/Mineclonia#127
No description provided.
Delete Branch "mineshaft-pig-spawners"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?