This repository has been archived by the owner. It is now read-only.

Player.php messed up #4239

Open
SOF3 opened this Issue Jul 1, 2016 · 3 comments

Comments

Projects
None yet
4 participants
@SOF3

SOF3 commented Jul 1, 2016

Issue description

Player.php is seriously messed up. The pocketmine\Player class has too many functions, which should be split up.

Functions

  • Entity (These should be done in the Human.php)
    • Movement
      • Collision
    • Health
    • Inventory holder
  • Metadatable (Fine, just redirect functions)
  • Player (a real human behind) (This is what a Player is for!)
    • With a name, skin, etc. (IPlayer)
    • Gameplay (gamemodes, chatting, achievements, etc.)
    • With respawn and spawn positions
    • Permissions, Banning
    • Anti-hack
    • With non-entity-generic data saving
    • Inventory opener (addWindow-related member functions)
    • World viewer (chunk loading, block update sending)
  • Client (through a connection) (Recommended: Create another class pocketmine\network\Client) (Possible enhancement: making it possible to have zero to multiple connections (Clients) for the same Player)
    • ACK/NACK handling
    • data packet handling and sending
    • login handling
    • chunk-sending
    • client-specific data
  • Miscellaneous (Move this to pocketmine\network\protocol\Info!

Steps to reproduce the issue

  1. Open the PocketMine-MP GitHub page to view source code
  2. Wait until your browser finishes loading.
  3. Enjoy the mess of code.

OS and versions

  • PocketMine-MP: any commit in the past year
  • PHP: any version applicable
  • OS: any OS that supports a browser that can browse GitHub

Crashdump, backtrace or other files

  • Player.php in source code
@PrimusLV

This comment has been minimized.

Show comment
Hide comment
@PrimusLV

PrimusLV Jul 1, 2016

@SOF3 is right but also remember there always will be something more to add, fix or remove. Impossible to satisfy everyone.

When one doors closes two more open.

PrimusLV commented Jul 1, 2016

@SOF3 is right but also remember there always will be something more to add, fix or remove. Impossible to satisfy everyone.

When one doors closes two more open.

@extremeheat

This comment has been minimized.

Show comment
Hide comment
@extremeheat

extremeheat Jul 2, 2016

You offering to "fix" it?

I'd say it's probably faster to leave it as it is and avoiding creating 20 new objects with all the added overhead.

extremeheat commented Jul 2, 2016

You offering to "fix" it?

I'd say it's probably faster to leave it as it is and avoiding creating 20 new objects with all the added overhead.

@iksaku

This comment has been minimized.

Show comment
Hide comment
@iksaku

iksaku Jul 3, 2016

Contributor

Multiple connections for the same Player? That could be harmful...

Contributor

iksaku commented Jul 3, 2016

Multiple connections for the same Player? That could be harmful...

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