Skip to content
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.

Sign up for GitHub

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

Fix bugs when selecting an area #140

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix bugs when selecting an area #140

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0250371
Fix bugs when selecting an area
Apr 20, 2020
0ee2cba
Bug fix when specifying a point in a multi-block block
Apr 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions Classes/ToolRegistrator.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,19 @@ function cToolRegistrator:BindAbsoluteTools()
return true
end

local LastRightClick = -math.huge
local function OnPlayerRightClick(a_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace)
local Succ, Message = GetPlayerState(a_Player).Selection:SetPos(a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, "Second")
if (not Succ) then
return false
end


-- Protection against the second packet when the second position is indicated
if ((os.clock() - LastRightClick) < 0.005) then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment here explaining why this is necessary would be useful (multiple packets come through in a single tick?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment here explaining why this is necessary would be useful (multiple packets come through in a single tick?)

In some cases, when the second position is indicated, the client sends 2 packets

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you check the a_BlockFace parameter? It's possible we can filter out the second packet if a_BlockFace is BLOCK_FACE_NONE.

Also, wouldn't it make more sense to put the check before setting the selection? Now the selection still gets set twice, but the second time the player isn't notified.

Copy link
Author

@ruqeg ruqeg May 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you check the a_BlockFace parameter? It's possible we can filter out the second packet if a_BlockFace is BLOCK_FACE_NONE.

Also, wouldn't it make more sense to put the check before setting the selection? Now the selection still gets set twice, but the second time the player isn't notified.

The problem is that we catch 3 packages, 2 of them are normal, and the third with BLOCK_FACE_NONE. Even with checking for BLOCK_FACE_NONE, 2 messages are displayed. Example:

  1. Normal
  2. BLOCK_FACE_NONE
  3. Normal

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I guess the os.clock() method is the simplest way to avoid this. It's also somewhat forwards compatible if Mojang decide to change the number of packets again.

return true
end
LastRightClick = os.clock()

a_Player:SendMessage(Message)
return true
end
Expand Down Expand Up @@ -290,12 +297,7 @@ end



local function LeftClickToolsHook(a_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_Action)
if (a_Action ~= 0) then
-- Left click is also called for other things like throwing items
return false
end

local function BreakingBlockHook(a_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_BlockType, a_BlockMeta)
local State = GetPlayerState(a_Player)
return State.ToolRegistrator:UseLeftClickTool(a_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_Player:GetEquippedItem().m_ItemType)
end
Expand All @@ -304,6 +306,7 @@ end




local function LeftClickToolsAnimationHook(a_Player, a_Animation)
-- In 1.8.x the left click has a value of 0, while in 1.7.x it's 1
local LeftClickAnimation = (a_Player:GetClientHandle():GetProtocolVersion() > 5) and 0 or 1
Expand All @@ -321,5 +324,5 @@ end

-- Register the hooks needed:
cPluginManager.AddHook(cPluginManager.HOOK_PLAYER_RIGHT_CLICK, RightClickToolsHook);
cPluginManager.AddHook(cPluginManager.HOOK_PLAYER_LEFT_CLICK, LeftClickToolsHook);
cPluginManager.AddHook(cPluginManager.HOOK_PLAYER_ANIMATION, LeftClickToolsAnimationHook);
cPluginManager.AddHook(cPluginManager.HOOK_PLAYER_BREAKING_BLOCK, BreakingBlockHook);
cPluginManager.AddHook(cPluginManager.HOOK_PLAYER_ANIMATION, LeftClickToolsAnimationHook);