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

Added ability to prevent mob spawns in areas #10

Closed
wants to merge 15 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@williamhatcher
Contributor

williamhatcher commented Aug 7, 2017

This should close #9

williamhatcher added some commits Aug 7, 2017

Updated Info.lua
Various spelling fixes
Added Version here
Used name & version from Info.lua
Also, removed unnecessary semisemicolons
Updated README.md
Via DumpInfo
Added AllowMobSpawning config
Removed unnecessary semicolons
Added HOOK_SPAWNING_MONSTER
Replaced MCS with cuberite
Removed unnecessary semicolons
Added CanMobSpawn to cPlayerAreas
This can later be modified to allow per area spawning
@williamhatcher

This comment has been minimized.

Contributor

williamhatcher commented Aug 16, 2017

You might find this useful, @HelenaKitty!

Also, could @madmaxoft or someone please review this? I feel like this is a good addition.

@NiLSPACE

This comment has been minimized.

Member

NiLSPACE commented Aug 16, 2017

It looks good, but why did you remove the semicolons? It's usually good practice to keep them since you can otherwise get issues like this. Lua likely has this issue a bit less, but it's still possible.

@madmaxoft

This comment has been minimized.

Member

madmaxoft commented Aug 16, 2017

@NiLSPACE JS originally required semicolons, but then gradually allowed the programmers to leave them out. By contrast, Lua has never needed semicolons (but supports having them). I'm personally glad that they were removed from this code, I don't think they should be used in Lua code.

@NiLSPACE

This comment has been minimized.

Member

NiLSPACE commented Aug 16, 2017

Really? I've always preferred to have semicolons if possible. In that case I do not have any objections.

end





--- Called by MCS when a player disconnects
--- Called by cuberite when a player disconnects

This comment has been minimized.

@madmaxoft

madmaxoft Aug 16, 2017

Member

Cuberite should have capital C

local z = a_Monster:GetPosZ()
local denySpawn = false
for pa_object in pairs(g_PlayerAreas) do
local area = g_PlayerAreas[pa_object]

This comment has been minimized.

@madmaxoft

madmaxoft Aug 16, 2017

Member

Why not use the efficient for _, area in pairs(g_PlayerAreas) do instead of the two lines above?

This comment has been minimized.

@williamhatcher

williamhatcher Aug 16, 2017

Contributor

I was not aware that g_PlayerAreas also had the area.

This comment has been minimized.

@madmaxoft

madmaxoft Aug 17, 2017

Member

It's just a rewrite of your code. Using for with pairs in Lua gives you both the key and value at the same time, your original code dismissed the value and then looked it up again in the map.

for pa_object in pairs(g_PlayerAreas) do
local area = g_PlayerAreas[pa_object]
if not(area:CanMobSpawn(x, z)) then
denySpawn = true

This comment has been minimized.

@madmaxoft

madmaxoft Aug 16, 2017

Member

Just return true here, no need to store the value at all.

Info.lua Outdated
@@ -18,7 +19,7 @@ g_PluginInfo =
An area is defined by the VIP by using a Wand item (by default, a stick with a meta 1) by left-clicking
and right-clicking in two opposite corners of the area, then issuing a /protection add command. Multiple
users can be allowed in a single area. There is no hardcoded limit on the number of areas or the number
users can be allowed in a single area. There is no hard coded limit on the number of areas or the number
of players allowed in a single area. Areas can overlap; in such a case, if a user is allowed in any one
of the overlapping areas, they are allowed to build / dig.

This comment has been minimized.

@madmaxoft

madmaxoft Aug 16, 2017

Member

The mob spawning restrictions should be explained here, too - if there is any area in which mob spawning is disabled for a block coord, mobs won't spawn at that coord (fix my broken English)

a_Plugin:SetName("ProtectionAreas");
a_Plugin:SetVersion(1);
a_Plugin:SetName(g_PluginInfo.Name)
a_Plugin:SetVersion(g_PluginInfo.Version)

This comment has been minimized.

@madmaxoft

madmaxoft Aug 16, 2017

Member

SetVersion accepts numbers only. Use tonumber.


return true;
)
LOG("Initialized " .. a_Plugin:GetName() .. " v." .. a_Plugin:GetVersion())

This comment has been minimized.

@madmaxoft

madmaxoft Aug 16, 2017

Member

Do we really need to log each plugin's initialization? I find it useless.

This comment has been minimized.

@williamhatcher

williamhatcher Aug 16, 2017

Contributor

Considering that the plugin manager doesn't tell you what plugins have been loaded, yes.

This comment has been minimized.

@madmaxoft

madmaxoft Aug 17, 2017

Member

The plugins console command gives you a list.

This comment has been minimized.

@williamhatcher

williamhatcher Aug 17, 2017

Contributor

I think most everyone does it because the Core plugin does it. I still think it is helpful to have it, but I'd be willing to remove it.

@williamhatcher

This comment has been minimized.

Contributor

williamhatcher commented Aug 16, 2017

@madmaxoft, should I go around a remove semicolons from all the lua code in Cuberite?

@williamhatcher

This comment has been minimized.

Contributor

williamhatcher commented Aug 16, 2017

I made requested changes.

@madmaxoft

This comment has been minimized.

Member

madmaxoft commented Aug 17, 2017

I see a potential problem with this implementation: the cPlayerAreas object only loads protected areas close to the player (at most g_AreaBounds blocks away from the player; see https://github.com/cuberite/ProtectionAreas/blob/master/Storage.lua#L190 ), while the mob can be further spawning away.

@madmaxoft

This comment has been minimized.

Member

madmaxoft commented Aug 17, 2017

Don't change all the semicolons just yet, let's focus on finishing the "useful" PRs first.

@williamhatcher

This comment has been minimized.

Contributor

williamhatcher commented Aug 17, 2017

Is there a global table or function that returns all the protection areas?

@williamhatcher

This comment has been minimized.

Contributor

williamhatcher commented Aug 17, 2017

A nice solution for this would be to create a function IsInArea that would return true if a given co-ord is in a player area. This would allow it to be used for several other flag related things.

@williamhatcher

This comment has been minimized.

Contributor

williamhatcher commented Aug 17, 2017

Does Cuberite spawn mobs in unloaded chunks? If not, then cPlayerAreas should only load areas in chunks that have been loaded.

@bearbin

This comment has been minimized.

Member

bearbin commented Aug 19, 2017

@williamhatcher why did you close this PR?

@williamhatcher

This comment has been minimized.

Contributor

williamhatcher commented Aug 20, 2017

That was an accident.

@williamhatcher

This comment has been minimized.

Contributor

williamhatcher commented Aug 21, 2017

I have an idea for implementing flags to allow mob spawning, pvp, etc.
I'll try adding that before working on this further.

@williamhatcher

This comment has been minimized.

Contributor

williamhatcher commented Aug 21, 2017

Important! @NiLSPACE, @madmaxoft, and @bearbin
I have to close this pull request and open a new one to move the source branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment