CivClassic / SimpleAdminHacks

Tools and backend for Citadel servers, including advanced inventory monitoring, active new player assistance, and other logging. Built for Paper 1.16.5
BSD 3-Clause "New" or "Revised" License
0 stars 19 forks source link

Add support for condensing mobs and merge all portal-spawned mobs into a single type. #82

Closed xFier closed 3 years ago

xFier commented 3 years ago

Added support for Mob Condensing, where spawns of a mob are reduced by a factor, but items dropped by that type of mob are increased by the same factor.

Stopped spawning Wither Skeletons and Ghasts from Nether Portals to replace them with Zombified Piglin variants.

Wither Skeletons will now be Zombie Piglins wearing a Wither Skull as a hat, and Ghasts will be Zombie Piglins holding a Ghast Tear in their offhand.

There should be no changes to game balance, aside from a reduction in mob counts.

PortalSpawnModifer requires no config changes. Mob Condenser requires a config in the following format: MobCondenser: enabled: true mobSpawnModifiers: ZOMBIFIED_PIGLIN: 0.10

The mobSpawnModifiers controls the entity type, and the new spawn rate, so in the above example Zombified Piglins will spawn 10x less, but drop 10x the items.

Diet-Cola commented 3 years ago

How does this handle a donkey with a chest attached with an inventory full of diamond blocks?

xFier commented 3 years ago

How does this handle a donkey with a chest attached with an inventory full of diamond blocks?

Probably how you'd expect it to. Not sure why you'd want to condense a donkey though

Quinndolin commented 3 years ago

How does this handle a donkey with a chest attached with an inventory full of diamond blocks?

a tale as old as time

okx-code commented 3 years ago

I don't understand why wither skeletons need to be zombie piglins now?

Virizion commented 3 years ago

I don't understand why wither skeletons need to be zombie piglins now?

mainly to be able to use piglin AI for farms cause it simplifies portal farms quite a bit

Diet-Cola commented 3 years ago

Just had another thought with this, wither skeles and pigmen can swap swords with players stuff right, so this could also multiple those drops

xFier commented 3 years ago

Just had another thought with this, wither skeles and pigmen can swap swords with players stuff right, so this could also multiple those drops

Piglin item pickup is disabled, which should stop that.

Ameliorate commented 3 years ago

Instead of held items, shouldn't this use something like PersistantDataContainers or NBT tags? Using PDCs or NBTs would completely prevent the possibility of dupe bugs.

wingzero54 commented 3 years ago

Where/ how does the material whitelist get set/added? Is it an additional config entry?

xFier commented 3 years ago

Where/ how does the material whitelist get set/added? Is it an additional config entry?

Yes, here’s an example config: MobCondenser: enabled: true mobSpawnModifiers: ZOMBIFIED_PIGLIN: 0.10 ZOMBIE: 0.50 materialModificationWhitelist:

Ameliorate commented 3 years ago

It's kind of a moot point now that it's merged, but I think that this PR will cause issues in the future that will lead to duping. Even though we have proved that it is impossible to dupe using this feature in the present, the civ codebase will change. I think that problems will especially arise in the mid-far future when we update to a new version of minecraft. We won't look at this code in context of the future changes, and it will break.

If this code breaks, it causes dupe bugs. If I was in charge of reviewing, I would have not merged this PR. We should be very careful when writing code that generates multiplied items, and this code is not very careful. As I stated above, this code should be using something unaccessable to the players, such as PersistantDataContainers or NBT tags.

wingzero54 commented 3 years ago

It's kind of a moot point now that it's merged, but I think that this PR will cause issues in the future that will lead to duping. Even though we have proved that it is impossible to dupe using this feature in the present, the civ codebase will change. I think that problems will especially arise in the mid-far future when we update to a new version of minecraft. We won't look at this code in context of the future changes, and it will break.

If this code breaks, it causes dupe bugs. If I was in charge of reviewing, I would have not merged this PR. We should be very careful when writing code that generates multiplied items, and this code is not very careful. As I stated above, this code should be using something unaccessable to the players, such as PersistantDataContainers or NBT tags.

I agree and it's something to keep a close eye on. If you come up with some PR's as improvements we would consider them

xFier commented 3 years ago

It's kind of a moot point now that it's merged, but I think that this PR will cause issues in the future that will lead to duping. Even though we have proved that it is impossible to dupe using this feature in the present, the civ codebase will change. I think that problems will especially arise in the mid-far future when we update to a new version of minecraft. We won't look at this code in context of the future changes, and it will break.

If this code breaks, it causes dupe bugs. If I was in charge of reviewing, I would have not merged this PR. We should be very careful when writing code that generates multiplied items, and this code is not very careful. As I stated above, this code should be using something unaccessable to the players, such as PersistantDataContainers or NBT tags.

I'm going to have to agree too, I'll likely make another PR addressing the use of PDRs and any other suggestions you have until you're happy with the state of the code.