Wertzui123 / BedrockClans

An advanced PocketMine-MP clan plugin with many creative features
GNU General Public License v3.0
30 stars 8 forks source link

EventAPI changes #20

Closed HimmelKreis4865 closed 2 years ago

HimmelKreis4865 commented 3 years ago

Hello Wertzui123,

To improve your API, please also call ClanJoinEvent if parameter is not null / ClanLeaveEvent if player was in a clan in case CPlayer::setClan(?Clan) was called ( https://github.com/Wertzui123/BedrockClans/blob/cd8839a3947d580cc335f42b27bc41e89fe8b7a0/src/Wertzui123/BedrockClans/BCPlayer.php#L63 )

This function's biggest use should be in the create subcommand if I saw it right.

A rename of PlayerChatEvent would be also great because it seems pretty much confusing comparing to PlayerChatEvent by PocketMine-MP.

Wertzui123 commented 3 years ago

A rename of PlayerChatEvent would be also great because it seems pretty much confusing comparing to PlayerChatEvent by PocketMine-MP.

I agree with you.

To improve your API, please also call ClanJoinEvent if parameter is not null / ClanLeaveEvent if player was in a clan in case CPlayer::setClan(?Clan) was called (

The purpose of this function is to update the clan property of the player internally. If you want to call the event etc., there is this method: https://github.com/Wertzui123/BedrockClans/blob/cd8839a3947d580cc335f42b27bc41e89fe8b7a0/src/Wertzui123/BedrockClans/BCPlayer.php#L157

HimmelKreis4865 commented 3 years ago

Then please rewrite the usage in "create" subcommand and also call the event. You could also ignore cancelled events but would be good to have this event called.

Wertzui123 commented 3 years ago

That's a question of design. Does the player really join the clan? I will think about it.

HimmelKreis4865 commented 3 years ago

He does actually. Creating lets him join the clan, which should be handled similarly to invite acceptions.

Wertzui123 commented 3 years ago

A rename of PlayerChatEvent would be also great because it seems pretty much confusing comparing to PlayerChatEvent by PocketMine-MP.

The event has been renamed to PlayerClanChatEvent in e6d0aac4caa8d33771bc64285addecbca1bc905d.

Wertzui123 commented 2 years ago

I think the PlayerClanJoinEvent should not be called on clan creation since there is also a ClanCreateEvent. If you still think it should, feel free to reopen the issue.