palra / univ-mmorpg

0 stars 0 forks source link

Move FightEvent to event.action package #13

Closed palra closed 9 years ago

palra commented 9 years ago

You should refactor class fr.univdevs.mmorpg.engine.event.FightEvent.FightEvent to fr.univdevs.mmorpg.engine.event.action.FightEvent. Not only FightEventis an invalid package name (it should be in lowercase), but it doesn't respect the event package naming convention set at #5.

palra commented 9 years ago

I re-read your code, and it seems you misunderstood how we should organise pour event classes in the event package.

An event is designed by its topic and its name. Events can be, then, represented in a tree, where events are agregated by their topic name. So, our events must be named that way : fr.univdevs.mmorpg.engine.event.<topic>.<name>Event

Don't forget that, for example, a CureEvent extends of ActionEvent, so it inherits of its topic. No need to create a constructor to initialize the topic of the event in ActionEvent, and so on CureEvent that doesn't need to have a constructor parameter for its event name, as it will be common to every instance of CureEvent (action.cure)

drattak commented 9 years ago

I know. Mistake For event classes, I just c/p classes already existing

Le ven. 12 juin 2015 00:47, Loïc Payol notifications@github.com a écrit :

I re-read your code, and it seems you misunderstood how we should organise pour event classes in the event package.

An event is designed by its topic and its name. Events can be, then, represented in a tree, where events are agregated by their topic name. So, our events must be named that way : fr.univdevs.mmorpg.engine.event..Event

Don't forget that, for example, a CureEvent extends of ActionEvent, so it inherits of its topic. No need to create a constructor to initialize the topic of the event in ActionEvent, and so on CureEvent that doesn't need to have a constructor parameter for its event name, as it will be common to every instance of CureEvent (action.cure)

— Reply to this email directly or view it on GitHub https://github.com/palra/univ-mmorpg/issues/13#issuecomment-111297779.