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

Windows 10 Edition support + whopping stack of inventory/hotbar bug fixes #1621

Merged
merged 38 commits into from Aug 17, 2016

Conversation

Projects
None yet
@dktapps
Contributor

dktapps commented Aug 8, 2016

Before we begin

I would like to ask for the approval of AT LEAST 4 reviewers before this PR is merged. This is a BIG update. I also would like testers from every platform if possible - Android, iOS, Windows, GearVR (If I missed any here, feel free to correct me).

Description

This PR has been a long time coming. But finally, it's here. Windows 10 Edition support, plus a gigantic stack of inventory and hotbar fixes all round.

Credits (apart from myself)

I owe thanks to @extremeheat for his P.O.C. patches for Win10 Edition. The FloatingInventory concept came from there, as did the concept for the Win10 crafting fixes. The original patches can be seen here.

Fixes

Windows 10 Edition

  • Crafting
  • Pressing Q to drop items one at a time works
  • Left-click-drag works beautifully (even despite the DDoS)
  • Issues with items not appearing in the hotbar have been mostly eliminated (I am aware that sometimes items obtained by /give do not show up, but that's not my fault.)

Pocket Edition

  • Chests: Remember the stupid bug where you'd try to transfer items to a chest and they'd reappear in your inventory? Say goodbye to it, it's gone and buried.
  • Anvils: The bug where they'd close immediately when used is gone.

All platforms

  • HOTBAR. The hotbar has been the single most annoying buggy thing in PM since forever. Hotbar now works pretty much flawlessly on PE and Windows 10 both.
  • Inventory: Every inventory spaz you care to name is fixed.
  • Held item sending: Thanks to @svilex for breaking it, but that is also fixed.
  • Armour equipment: Bugs where armour would vanish have been fixed.
  • Creative: Hotbar bugs, chest bugs, crafting bugs, bugs, bugs, bugs - fixed.

Changes

  • An enormously powerful new inventory anti-cheat system. It's next to impossible to dupe items, hack items in, lose items, etc. (#1626) Say goodbye to players using hacks to cheat items in.
  • FloatingInventory. This is used to store items in-between transactions. If you remove and item from your inventory, it goes into here. The FloatingInventory is also currently used for Win10 crafting materials due to Win10 bugs (Windows 10 doesn't tell the host what's in the crafting grid: https://bugs.mojang.com/browse/MCPE-16366).
  • Inventory transaction processing now makes use of a transaction queue. Transactions sent by the client are queued for execution. Failed transactions are retried up to 5 times. This fixes an enormous stack of issues with chests in PE.
  • PlayerInventory::setHotbarSlotIndex($index, $slot) has been deprecated due to the massive issues it causes with Windows 10 Edition. Hotbar slot linkages will now only be changed if directly requested by the client in a MobEquipmentPacket. The reason for this is that on Win10 hotbar slots are ALWAYS linked to the first 9 slots. (Blame Mojang for allowing them to be changed on Win10.)
  • Enchanting tables now open inventories per-player and do not have an inventory tied to their tile. This is because enchanting tables do not have permanent inventories, and setting enchanting table contents server-side causes clients to crash.
  • Anti-cheat inventory transaction processing can now be disabled in genisys.yml. This is required for enchanting tables and anvils to work on Win10 currently (#blamemojang for Win10 bugs).
  • InventoryTransactionEvent has been removed since it is impossible to pair transactions in Win10.
  • Inventory custom size options have also been removed due to crashes on Win10 Edition.

Known issues

  • Anvils and enchanting tables do not currently work on Windows 10 Edition unless inventory.allow-cheats is set to true in genisys.yml. This is because of bugs in Win10 which make it impossible to process anti-cheats. Fixes for this will be incoming in the near future.
  • Crafting: Some recipes cannot be placed anywhere in the crafting grid like they can in vanilla. My next job is an upcoming crafting rewrite in which I will attempt to resolve this issue.
  • Occasional random segmentation faults have been spotted. I believe this to be a pthreads issue. If anyone can verify this or find the cause, it would be much appreciated.

Things all devs should be aware of

  • Hacks to fix client bugs should be superficial and easy to remove once the bug is fixed.
  • Packet bouncing is the root of all evil.

Reason to modify

Support Windows 10 Edition Beta, Gear VR Edition and fix all of the bugs mentioned above.

Tests & Reviews

This update has been in testing for weeks. I've changed a LOT of things for this, and it's impossible for me to test every tiny little thing. I have however done very comprehensive testing and I believe that this update is now ready for production use.

Please review things below

I would like to require the approval of AT LEAST 4 reviewers before this PR is merged. As I said, I've made a LOT of changes, and it's impossible for me to test every device. I have tested primarily on Windows 10 Edition and PE (Android). I'd like to ask for iOS testers and also Gear VR testers.

Let the reviewing begin!

Related bugs

Close #1666

@Muqsit

This comment has been minimized.

Show comment
Hide comment
@Muqsit

Muqsit Aug 8, 2016

Member

OMG!

Member

Muqsit commented Aug 8, 2016

OMG!

Show outdated Hide outdated src/pocketmine/Server.php
@@ -2107,7 +2107,7 @@ public function __construct(\ClassLoader $autoloader, \ThreadedLogger $logger, $
}
$this->generateRecipeList();
$this->logger->warning("This build is from the Win10 Edition Beta development branch and is NOT intended for production use! Only use this build for testing purposes.");

This comment has been minimized.

@robske110

robske110 Aug 8, 2016

Contributor

did you get rid of this commit? Maybe just DROP it :)

@robske110

robske110 Aug 8, 2016

Contributor

did you get rid of this commit? Maybe just DROP it :)

This comment has been minimized.

@dktapps

dktapps Aug 8, 2016

Contributor

Already did locally, doing lots of squashing now

@dktapps

dktapps Aug 8, 2016

Contributor

Already did locally, doing lots of squashing now

@extremeheat

This comment has been minimized.

Show comment
Hide comment
@extremeheat

extremeheat Aug 8, 2016

Looking good so far, no major issues I've encountered. The issue where some items don't appear until the client opens the inventory persist from what I've seen. Probably an issue with how W10E handles ContainerSetContentPacket's hotbar mappings.

Still have to look into how vanilla servers send the crafting data but I can imagine the wrong recipe IDs being sent are just client-sided issues.

extremeheat commented Aug 8, 2016

Looking good so far, no major issues I've encountered. The issue where some items don't appear until the client opens the inventory persist from what I've seen. Probably an issue with how W10E handles ContainerSetContentPacket's hotbar mappings.

Still have to look into how vanilla servers send the crafting data but I can imagine the wrong recipe IDs being sent are just client-sided issues.

@ClockProgramming

This comment has been minimized.

Show comment
Hide comment
@ClockProgramming

ClockProgramming Aug 8, 2016

Looking far good, I can test this if I have free time

On Aug 9, 2016 3:04 AM, "extremeheat" notifications@github.com wrote:

Looking good so far, no major issues I've encountered. The issue where
some items don't appear until the client opens the inventory persist from
what I've seen. Probably an issue with how W10E handles
ContainerSetConentPacket's hotbar mappings.

Still have to look into how vanilla servers send the crafting data but I
can imagine the wrong recipe IDs being sent are just client-sided issues.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1621 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/APv_xZ0n5iHqLn1TDdT0udn2oZBqUjYsks5qd32tgaJpZM4JfRNK
.

Looking far good, I can test this if I have free time

On Aug 9, 2016 3:04 AM, "extremeheat" notifications@github.com wrote:

Looking good so far, no major issues I've encountered. The issue where
some items don't appear until the client opens the inventory persist from
what I've seen. Probably an issue with how W10E handles
ContainerSetConentPacket's hotbar mappings.

Still have to look into how vanilla servers send the crafting data but I
can imagine the wrong recipe IDs being sent are just client-sided issues.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1621 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/APv_xZ0n5iHqLn1TDdT0udn2oZBqUjYsks5qd32tgaJpZM4JfRNK
.

@dschwartz783

This comment has been minimized.

Show comment
Hide comment
@dschwartz783

dschwartz783 Aug 8, 2016

Contributor

Haven't noticed anything bad, but I have noticed lots of good. Seems like a solid PR so far, based on my tests.

Contributor

dschwartz783 commented Aug 8, 2016

Haven't noticed anything bad, but I have noticed lots of good. Seems like a solid PR so far, based on my tests.

@ZedCee

This comment has been minimized.

Show comment
Hide comment
@ZedCee

ZedCee Aug 8, 2016

Member

Solid work. I've been using this as my server for a few days now for many of it's bug fixes.

I'll begin deeper testing on iOS tonight and can wrap my girlfriend into testing for Android, as soon as possible.

Member

ZedCee commented Aug 8, 2016

Solid work. I've been using this as my server for a few days now for many of it's bug fixes.

I'll begin deeper testing on iOS tonight and can wrap my girlfriend into testing for Android, as soon as possible.

@gaoyichuan

This comment has been minimized.

Show comment
Hide comment
@gaoyichuan

gaoyichuan Aug 8, 2016

Member

Seems interesting. I'll test it this morning

Member

gaoyichuan commented Aug 8, 2016

Seems interesting. I'll test it this morning

@FaanMaario

This comment has been minimized.

Show comment
Hide comment
@FaanMaario

FaanMaario Aug 9, 2016

Tested, nothing bad at all, and the Win10 bugs seems to be fixed.

It works fine :D

Tested, nothing bad at all, and the Win10 bugs seems to be fixed.

It works fine :D

@Muqsit

This comment has been minimized.

Show comment
Hide comment
@Muqsit

Muqsit Aug 9, 2016

Member

Same, tested. Can't find bugs on Win10 and PE.
I don't get segmentation faults anywhere except when stopping or restarting (DO_LOOP ing) the server, which is quite normal for me.

Member

Muqsit commented Aug 9, 2016

Same, tested. Can't find bugs on Win10 and PE.
I don't get segmentation faults anywhere except when stopping or restarting (DO_LOOP ing) the server, which is quite normal for me.

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 9, 2016

Contributor

@extremeheat I've noticed that myself, but only on player join. I have however discovered something else since I made this PR that may fix the issue.

As for crafting, there are a variety of server-side problems with crafting that I believe would cause it. My next job is a total crafting rewrite.

Contributor

dktapps commented Aug 9, 2016

@extremeheat I've noticed that myself, but only on player join. I have however discovered something else since I made this PR that may fix the issue.

As for crafting, there are a variety of server-side problems with crafting that I believe would cause it. My next job is a total crafting rewrite.

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 9, 2016

Contributor

Incidentally, I recommend deleting your <player>.dat file, or join as a new user. I made some tweaks to the inventory save format, so using an old one might cause some issues. @extremeheat since I recreated my profile, I haven't been able to reproduce the hotbar problems. (except very occasionally with the /give command)

Contributor

dktapps commented Aug 9, 2016

Incidentally, I recommend deleting your <player>.dat file, or join as a new user. I made some tweaks to the inventory save format, so using an old one might cause some issues. @extremeheat since I recreated my profile, I haven't been able to reproduce the hotbar problems. (except very occasionally with the /give command)

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 9, 2016

Good job :)

ghost commented Aug 9, 2016

Good job :)

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 9, 2016

Contributor

Problems with hackers cheating items in? I forgot to mention that my new anti-cheat system puts a stop to that. :D

Contributor

dktapps commented Aug 9, 2016

Problems with hackers cheating items in? I forgot to mention that my new anti-cheat system puts a stop to that. :D

@FaanMaario

This comment has been minimized.

Show comment
Hide comment
@FaanMaario

FaanMaario Aug 9, 2016

@dktapps Oh yeah I just tested this, and your anti-cheat system is awesome !

Good job :D

@dktapps Oh yeah I just tested this, and your anti-cheat system is awesome !

Good job :D

@drewsucksatlife

This comment has been minimized.

Show comment
Hide comment
@drewsucksatlife

drewsucksatlife Aug 10, 2016

Contributor

Seems amazing. I wish I could test it, but my VPS host isn't working...

Contributor

drewsucksatlife commented Aug 10, 2016

Seems amazing. I wish I could test it, but my VPS host isn't working...

@ZedCee

This comment has been minimized.

Show comment
Hide comment
@ZedCee

ZedCee Aug 10, 2016

Member

Gave a decent run at it tonight. So far, so good. A real solid piece of work.
There was a moment where the anti-cheat system may have been a bit voracious for me, but have not been able to reproduce. I tried several different attempts with anit-cheat on and off. Taking two similar (possibly enchanted) items and placing them on an anvil then promptly exiting the menu only dropped one of the items. I could not reproduce this issue.

However one easily reproducible error, but likely trivial for the time being, has to do with equipping items from the creative menu if they preexist within the inventory. Sometimes, usually within the first few slots, two adjacent slots fill with said item.

Another quick review when my girlfriend's up to help test out another variety of transactions and gameplay between iOS and Android.

Member

ZedCee commented Aug 10, 2016

Gave a decent run at it tonight. So far, so good. A real solid piece of work.
There was a moment where the anti-cheat system may have been a bit voracious for me, but have not been able to reproduce. I tried several different attempts with anit-cheat on and off. Taking two similar (possibly enchanted) items and placing them on an anvil then promptly exiting the menu only dropped one of the items. I could not reproduce this issue.

However one easily reproducible error, but likely trivial for the time being, has to do with equipping items from the creative menu if they preexist within the inventory. Sometimes, usually within the first few slots, two adjacent slots fill with said item.

Another quick review when my girlfriend's up to help test out another variety of transactions and gameplay between iOS and Android.

@File14 File14 referenced this pull request Aug 10, 2016

Closed

Bug armour #782

@dog194

This comment has been minimized.

Show comment
Hide comment
@dog194

dog194 Aug 10, 2016

Collaborator

amazing, but i don have MC win10 edison. :(

Collaborator

dog194 commented Aug 10, 2016

amazing, but i don have MC win10 edison. :(

@dschwartz783

This comment has been minimized.

Show comment
Hide comment
@dschwartz783

dschwartz783 Aug 10, 2016

Contributor
Contributor

dschwartz783 commented Aug 10, 2016

@ishitatsuyuki

This comment has been minimized.

Show comment
Hide comment
@ishitatsuyuki

ishitatsuyuki Aug 10, 2016

Member

Seems most people are saying OK with this, and no serious bugs exist. How about deploying this and fix when a bug report kicks in?

Member

ishitatsuyuki commented Aug 10, 2016

Seems most people are saying OK with this, and no serious bugs exist. How about deploying this and fix when a bug report kicks in?

@NycuRO

This comment has been minimized.

Show comment
Hide comment

NycuRO commented Aug 10, 2016

+1 omg best @dktapps

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 10, 2016

Contributor

@dog194 Lots of improvements all round, not just for Win10. I rewrote various things and the results are rather incredible.
@ZedCee Anvils are weird all round. That's next on my list of things to do, along with crafting. And as for the thing with creative... um, I'll look into it. It's probably something to do with the new hotbar link setup.
@ishitatsuyuki If you think that's okay, by all means. :D

Contributor

dktapps commented Aug 10, 2016

@dog194 Lots of improvements all round, not just for Win10. I rewrote various things and the results are rather incredible.
@ZedCee Anvils are weird all round. That's next on my list of things to do, along with crafting. And as for the thing with creative... um, I'll look into it. It's probably something to do with the new hotbar link setup.
@ishitatsuyuki If you think that's okay, by all means. :D

@House-Kitty-Mew

This comment has been minimized.

Show comment
Hide comment
@House-Kitty-Mew

House-Kitty-Mew Aug 10, 2016

those fixes man.

those fixes man.

@drewsucksatlife

This comment has been minimized.

Show comment
Hide comment
@drewsucksatlife

drewsucksatlife Aug 10, 2016

Contributor

Everyone at my UHC server don't see any issues with it! Only problem I have encountered so far was every now and then a player .dat file has to be deleted or the server reboots.

Contributor

drewsucksatlife commented Aug 10, 2016

Everyone at my UHC server don't see any issues with it! Only problem I have encountered so far was every now and then a player .dat file has to be deleted or the server reboots.

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 10, 2016

Contributor

@TheAppleGamer Can you provide any more detail about this? And yes, some of the inventory save format has been modified. Older versions may encounter issues.

Contributor

dktapps commented Aug 10, 2016

@TheAppleGamer Can you provide any more detail about this? And yes, some of the inventory save format has been modified. Older versions may encounter issues.

@drewsucksatlife

This comment has been minimized.

Show comment
Hide comment
@drewsucksatlife

drewsucksatlife Aug 10, 2016

Contributor

@dktapps I reset the .dats because I saw what you said. I then install the branch and everything was working okay. Then my friend came to test and he was able to join the first time and then he relogged and every time he went to rejoin the server would reboot. I reset his .dat file again and it started working

Contributor

drewsucksatlife commented Aug 10, 2016

@dktapps I reset the .dats because I saw what you said. I then install the branch and everything was working okay. Then my friend came to test and he was able to join the first time and then he relogged and every time he went to rejoin the server would reboot. I reset his .dat file again and it started working

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 10, 2016

Contributor

@TheAppleGamer all right, thanks. I haven't encountered anything like that, I'll look into it. Has it happened multiple times or just that once?

Contributor

dktapps commented Aug 10, 2016

@TheAppleGamer all right, thanks. I haven't encountered anything like that, I'll look into it. Has it happened multiple times or just that once?

@drewsucksatlife

This comment has been minimized.

Show comment
Hide comment
@drewsucksatlife

drewsucksatlife Aug 10, 2016

Contributor

@dktapps Only one time so far, but I haven't had a UHC at all today. I will update you on it if it occurs again.

Contributor

drewsucksatlife commented Aug 10, 2016

@dktapps Only one time so far, but I haven't had a UHC at all today. I will update you on it if it occurs again.

@ZedCee

This comment has been minimized.

Show comment
Hide comment
@ZedCee

ZedCee Aug 10, 2016

Member

Could this have to do with the limitation on slots?

Member

ZedCee commented Aug 10, 2016

Could this have to do with the limitation on slots?

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 10, 2016

Contributor

@ZedCee I doubt it. The current NBT format allows anything up to 91 slots. The only changes I made to the save format were to the hotbar data saving.

Contributor

dktapps commented Aug 10, 2016

@ZedCee I doubt it. The current NBT format allows anything up to 91 slots. The only changes I made to the save format were to the hotbar data saving.

@ZedCee

This comment has been minimized.

Show comment
Hide comment
@ZedCee

ZedCee Aug 10, 2016

Member

But removed the genisys.yml option for more? Or is that just waiting a future update...?

Member

ZedCee commented Aug 10, 2016

But removed the genisys.yml option for more? Or is that just waiting a future update...?

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 10, 2016

Contributor

@ZedCee I removed it because a) crashes on Win10, and b) it's pointless. Plugins can do that, and in a cleaner fashion too. There should be no size-related crashes.

Contributor

dktapps commented Aug 10, 2016

@ZedCee I removed it because a) crashes on Win10, and b) it's pointless. Plugins can do that, and in a cleaner fashion too. There should be no size-related crashes.

@dschwartz783

This comment has been minimized.

Show comment
Hide comment
@dschwartz783

dschwartz783 Aug 10, 2016

Contributor

@dktapps Apparently some of our users are reporting that when people die, they drop a large pile of pick axes... might be plugin-related, might not.

Contributor

dschwartz783 commented Aug 10, 2016

@dktapps Apparently some of our users are reporting that when people die, they drop a large pile of pick axes... might be plugin-related, might not.

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 10, 2016

Contributor

@dschwartz783 what? I'll test that... lmao

Contributor

dktapps commented Aug 10, 2016

@dschwartz783 what? I'll test that... lmao

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 10, 2016

Contributor

Goddammit. I took a gamble on that one to fix a tiny bug and it didn't pay off. aaargh

Contributor

dktapps commented Aug 10, 2016

Goddammit. I took a gamble on that one to fix a tiny bug and it didn't pay off. aaargh

@ZedCee

This comment has been minimized.

Show comment
Hide comment
@ZedCee

ZedCee Aug 10, 2016

Member

@dktapps I'll ask you more regarding slots on Mattermost; it's trivial at the moment.
@dschwartz783 Reminds me of the bug that caused you to drop a random assortment of all your picked up drops since login when you closed a chest.... Oh, would you look at that... It was. Just became oddly specific to reproduce.


I have thoroughly tested chests, anvils, and enchanting tables, and can only say this is a far-cry better than the present state of master. Just about any errors that come with it are trivial compared to the power at which it functions over the present production model.

I was able to produce a duplication with chest transactions. It can be reproduced but its not necessarily an easy one to do so. Anti-cheat makes no difference.
Load a chest with full stacks and commit transaction atrocities on it between two players. Just keep randomly opening and closing the chest, taking and adding different amounts of items. Eventually you may see one or two block duplicates; in my case it was the same each time; one update! block became an update!2 block or added an extra block, and I ended up with an extra portal block. On one attempt I lost an enchantment table.

As for Anvils and Enchantment Tables transactions, I found zero fault in the current function paradigm. Simultaneous transactions caused zero issues with anti-cheat off or on.

Finally, a solution for all our hotbar woes! There's not a lot I can say I haven't already on this note. It functions right. The way it should. It got a fairly good shake down and I can say without a doubt, if merged on the fixes for the hotbar alone, new bugs would be tolerable. (see my comment above)

To summarize, most, if not all, of our transaction issues seem to be fixed with these changes. Functions well on multiple interacting platforms and errors and debugs are at a minimum (sometimes an anvil, not renamed, debug pops up, but most times the item still ends up renamed). I, personally, give this a strong plus one.


Tested. Further testing required for new fixes.

OS and versions

  • Genisys: f610b9c
  • PHP: 7.0.9
  • Server OS: Arch Linux
  • Game version: 0.15.4 iOS and Android
Member

ZedCee commented Aug 10, 2016

@dktapps I'll ask you more regarding slots on Mattermost; it's trivial at the moment.
@dschwartz783 Reminds me of the bug that caused you to drop a random assortment of all your picked up drops since login when you closed a chest.... Oh, would you look at that... It was. Just became oddly specific to reproduce.


I have thoroughly tested chests, anvils, and enchanting tables, and can only say this is a far-cry better than the present state of master. Just about any errors that come with it are trivial compared to the power at which it functions over the present production model.

I was able to produce a duplication with chest transactions. It can be reproduced but its not necessarily an easy one to do so. Anti-cheat makes no difference.
Load a chest with full stacks and commit transaction atrocities on it between two players. Just keep randomly opening and closing the chest, taking and adding different amounts of items. Eventually you may see one or two block duplicates; in my case it was the same each time; one update! block became an update!2 block or added an extra block, and I ended up with an extra portal block. On one attempt I lost an enchantment table.

As for Anvils and Enchantment Tables transactions, I found zero fault in the current function paradigm. Simultaneous transactions caused zero issues with anti-cheat off or on.

Finally, a solution for all our hotbar woes! There's not a lot I can say I haven't already on this note. It functions right. The way it should. It got a fairly good shake down and I can say without a doubt, if merged on the fixes for the hotbar alone, new bugs would be tolerable. (see my comment above)

To summarize, most, if not all, of our transaction issues seem to be fixed with these changes. Functions well on multiple interacting platforms and errors and debugs are at a minimum (sometimes an anvil, not renamed, debug pops up, but most times the item still ends up renamed). I, personally, give this a strong plus one.


Tested. Further testing required for new fixes.

OS and versions

  • Genisys: f610b9c
  • PHP: 7.0.9
  • Server OS: Arch Linux
  • Game version: 0.15.4 iOS and Android

@dktapps dktapps added wip and removed wip labels Aug 10, 2016

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 10, 2016

Contributor

Okay, fixed. Please test again. @dschwartz783 thanks for digging that up.

Contributor

dktapps commented Aug 10, 2016

Okay, fixed. Please test again. @dschwartz783 thanks for digging that up.

dktapps added some commits Jul 13, 2016

Fix some chest update bugs
Not sure how to deal with the delay - it seems to be a requirement.
Implemented DropItemTransaction (read desc)
Allows item drops to be processed as inventory transactions. This allows PE players to drop items directly from the hotbar without any bugs or inconsistencies.
Remove unused interface
This interface is unnecessary and unused.
Move inventory restore to PlayerInventory constructor
This removes the final use of the deprecated setHotbarSlotIndex() and also completely fixes PE hotbar issues with saving, restoring, swapping slots and selecting items.
Now flawlessly working left-click-drag, and removed some old code.
Remove remnants of old transaction processor. Remove unused InventoryTransactionEvent (it's impossible to do this now because inventory transactions cannot be paired.)
Added options to disable verified transaction processing
This allows Win10 to use anvils without bugs.
Hotbar slot linkage now resets on death
Add missing hotbar constant to ContainerSetContentPacket

$pk->hotbar is only ever used when the windowid is 7a. Any other time,
just send 0.
Fixed player equipment update bugs
MobEquipmentPacket is sent _before_ the ContainerSetSlotPacket, so the
server sends to viewers what the _last_ item was. Now resending the held
item when the held slot is updated.

Always send armour slots to viewers when updated, regardless of the circumstance
Refactor CraftingInventory as FloatingInventory
CraftingInventory will be needed when PE gets desktop crafting and Win10
gets crafting fixed.

Clean up lots of spam and mess
Fix hotbar phantom items, remove inventory size option due to W10 cra…
…shes

Fix some empty slot mapping bugs and fix PE inventory size a cleaner way without affecting the entire inventory functionality.
Moral of the story: Hacks for client bugs should be superficial.

Remove comment

Hacks to fix client bugs should not affect functionality!

98fcede
Slightly cleaner crafting
This doesn't need to be executed for desktop crafting
Dropped item pickup fixes and some small tweaks
Dropped item pickup is handled by the client when it receives the
TakeItemEntityPacket, therefore the dropped items should be added to the
_floating_ inventory where they will await the accompanying slot change,
which the client will _also_ send.
@ZedCee

This comment has been minimized.

Show comment
Hide comment
@ZedCee

ZedCee Aug 17, 2016

Member

@iTXTech/trusted Be really nice to start considering these changes for future PRs soon +1

Member

ZedCee commented Aug 17, 2016

@iTXTech/trusted Be really nice to start considering these changes for future PRs soon +1

@ishitatsuyuki

This comment has been minimized.

Show comment
Hide comment
@ishitatsuyuki

ishitatsuyuki Aug 17, 2016

Member

Let's take action. Code review +1

Approved with PullApprove

Member

ishitatsuyuki commented Aug 17, 2016

Let's take action. Code review +1

Approved with PullApprove

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 17, 2016

Contributor

@ZedCee The PullApprove resets when I push, would you test and review again with the latest commit?
Just fixes a weird item pickup bug.

Contributor

dktapps commented Aug 17, 2016

@ZedCee The PullApprove resets when I push, would you test and review again with the latest commit?
Just fixes a weird item pickup bug.

@ZedCee

This comment has been minimized.

Show comment
Hide comment
@ZedCee

ZedCee Aug 17, 2016

Member

Tested. +1

OS and versions

  • Genisys: 4b26b36
  • PHP: 7.0.9 (from Arch)
  • Server OS: Arch Linux
  • Game version: 0.15.6 iOS and Android

Approved with PullApprove

Member

ZedCee commented Aug 17, 2016

Tested. +1

OS and versions

  • Genisys: 4b26b36
  • PHP: 7.0.9 (from Arch)
  • Server OS: Arch Linux
  • Game version: 0.15.6 iOS and Android

Approved with PullApprove

@dktapps

This comment has been minimized.

Show comment
Hide comment
@dktapps

dktapps Aug 17, 2016

Contributor

Tested on Win10, no issues to speak of. Shoot first, ask questions later.

Contributor

dktapps commented Aug 17, 2016

Tested on Win10, no issues to speak of. Shoot first, ask questions later.

@dktapps dktapps merged commit 78f41ed into master Aug 17, 2016

2 checks passed

code-review/pullapprove Approved by ishitatsuyuki, ZedCee
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dktapps dktapps deleted the w10-done-properly branch Aug 17, 2016

@robske110 robske110 referenced this pull request Aug 22, 2016

Merged

[WIP] Win10 fixes #682

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