Fix spawner delay feature #3239
Conversation
Essentials/src/com/earth2me/essentials/commands/Commandspawner.java
Outdated
Show resolved
Hide resolved
|
NMS should really be in providers - unfortunately the current naming convention of providers makes it difficult to know where to put them. |
|
Yeah I took a glance at the existing provider system but it wasn't pretty. If I can get some advice with refactoring this spawner code into its own provider that would be neat. Another confusing thing is it seems like the "SpawnerProvider" doesn't really have anything to do with spawners either, so there's that... |
|
As far as I can recall, the current "spawner providers" are for setting the entity type on stacks of spawners when they're spawned in. Perhaps the current spawner providers could be renamed to |
|
@md678685 Sorry for the lack of updates, I did actually figure this out while looking deeper at the spawner provider code. It is indeed intended for accessing the entity type of a spawner ItemStack. My intention is to refactor the code of this PR into a provider (either the existing one, or split into its own as your are suggesting) to allow backwards compatibility with versions that don't have a complete CreatureSpawner API. Perhaps I'm currently focusing on having #3279 resolved before I continue working on this (so I don't have to make further changes to the code). |
Sounds good. |
nms/UpdatedMetaProvider/src/net/ess3/nms/updatedmeta/BukkitSpawnerBlockProvider.java
Outdated
Show resolved
Hide resolved
|
Ready again for review - all issues resolved and tested |
Closes EssentialsX#1332 The spawner delay feature has been broken in Essentials for as long as anyone can remember. The reasons for this are mentioned in the issue above. This PR fixes this by changing the command to utilize new API for setting the minimum and maximum spawn delay on spawners. This API was added in 1.12.2, so all supported versions before that (1.8.8 thru 1.12.1) require NMS to function properly. I'm aware that Essentials avoids NMS for maintainability reasons, however that should not be of much concern here since all versions 1.12.2 and later are going to be using the Bukkit API. Hence, no NMS updates will be necessary. Also let me know if you want the NMS code refactored somewhere else. I saw the net.ess3.nms packages, but I wasn't sure where this would fit into the organisation of that. Tested on: 1.8.8, 1.9.4, 1.10.2, 1.11.2 (NMS) 1.12.2, 1.15.2 (Bukkit API)
Closes #1332
The spawner delay feature has been broken in Essentials for as long as anyone can remember. The reasons for this are mentioned in the issue above.
This PR fixes this by changing the command to utilize new API for setting the minimum and maximum spawn delay on spawners. This API was added in 1.12.2, so all supported versions before that (1.8.8 thru 1.12.1) require NMS to function properly. I'm aware that Essentials avoids NMS for maintainability reasons, however that should not be of much concern here since all versions 1.12.2 and later are going to be using the Bukkit API. Hence, no NMS updates will be necessary.
Also let me know if you want the NMS code refactored somewhere else. I saw the net.ess3.nms packages, but I wasn't sure where this would fit into the organisation of that.
Tested on:
1.8.8, 1.9.4, 1.10.2, 1.11.2 (NMS)
1.12.2, 1.15.2 (Bukkit API)