methusalah / OpenRTS

Real-Time Strategy game 3D engine coded in pure java
MIT License
1.29k stars 151 forks source link

Architecture Question #67

Closed meltzow closed 9 years ago

meltzow commented 9 years ago

Hi,

what are the idea and the tasks of the following classes: model.Commander.java model.Model.java model.Reporter.java model.ReportEventListener.java I need more background to understand it right. Until now, it looks a little bit confusing. Thx.

methusalah commented 9 years ago

The role of Commander is to create group of units and give orders. It as been created to keep the BattlefieldController a simple binding and input's interpretation class.

The role of the reporter (and the report event listener interface that link to it) is more confuse, as it has been done to give units info to the GUI drawer. Because de GUI drawer shouldn't know much of the group, units, etc... the reporter intended to create a resume of the info to be drawn on the GUI. For now, this is only used for the center panel of the battlefield GUI and honestly, it may need a refactor ^^.

The model instanciates a battlefield and the builder library from files and user commands. It also check for data change and update the library. Historicaly, it has holded many roles (all, in facts) for a time, before more dedicated class were created below it

Le sam. 16 mai 2015 à 14:58, Mario Meltzow notifications@github.com a écrit :

Hi,

what are the idea and the tasks of the following classes: model.Commander.java model.Model.java model.Reporter.java model.ReportEventListener.java I need more background to understand it right. Until now, it looks a little bit confusing. Thx.

— Reply to this email directly or view it on GitHub https://github.com/methusalah/OpenRTS/issues/67.

meltzow commented 9 years ago

ok, so it sounds like commander and model can be singletons, can't they? Only one commander (class instance) and model is needed at runtime. And why are the commander inside the model? The model is something like a dataholder entity. It loads, save the data (example the battlefield). It will be much easier if these classes are singletons. Any doubts of it? If no, then I will do refactor it.

methusalah commented 9 years ago

Model is kind of an abstract container of the MVC model. It is clearly a singleton, and the BuilderLibrary too. For Commander, Reporter or Battlefield, they indeed have only once instance at a time, but this instance may change at runtime (a new map is loaded).

So I would suggest to make the Model the only one singleton, which contains the other single instance classes like BuilderLibrary, Commander or even Battlefield.

If you can reach the model, you can reach everything with 'model.commander'. This way, you insure to have the last and up to date instance of what you need, everywhere and everytime. It is a little more verbose, though.

What do you think?

The fact is that I'm not very confortable with singletons, for they give wild access to their scope from everywhere. I fear that because it may lead to design aberations. But let's try if you think it's good !

meltzow commented 9 years ago

Currently we have no understandable wild access the to battlefield, commander and the Model. A lot of classes get them inside the constructor and handle on it. Mostly of fields are public, this means all filed are accessable from everywhere. Let my try... I am sure, it will be better understandable for a outstanding person(developer).

meltzow commented 9 years ago

It's not finished but. but initial step is done. @methusalah your ideas are great and it's very similar to the EntitySystem-architeture (see https://github.com/methusalah/OpenRTS/issues/62), your are using Managers (same like systems in entitysystem ) and you have a need to communicate with events (just at the beginning now). The component-thing is not realized.

But I can close this issue now. @methusalah in eclipse right click on the project -> gradle -> refreh all (get the new dependencies from gradle)