I consider it a bug if ingame screens and dialogs which are meant to be singleton can have multiple instances at the same time.
E.g. if you press F9 many times you will receive as many "Religion" advisor screens, which doesn't make sense.
That should be easy to fix, provided that we can agree on which panels might logically be displayed more than once. In the distant past, most (all?) panels were singletons, but this was abandoned for the sake of simplicity and better localization.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I believe I was able to solve this. I turned all the classes extending ReportPanel and turned them into a Singleton Pattern Design so that only one panel of that instance can be shown at a time.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
and BTW, what is the point of the doubled semi-colons? There are several instances.
Lots of duplicate code there in the Report*Panels. Can it be pushed up the class hierarchy? Or do we need a SingletonReportPanel class between them and ReportPanel?
Please do not use tabs. They are really annoying unless everyone has exactly the same indentation conventions and editor settings.
Speaking of which, please be a more vigilant about following the standard Java coding conventions. They do make life easier in the long run.
And yes, I realize the current FreeCol codebase is not exactly perfect there:-).
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Wow, I can't believe my clumsy hands actually pressed two semicolons on so many statements.
On another note, I'm not sure quite sure how I should go about pushing all ReportPanels into a SingletonReportPanel hierarchy as I am not that good at design. I'll have to leave each class as separate entities for now.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I looked over it, but it still bugged me that we have to create all these bogus "instance" variables --- after all, if we are displaying the panel an instance already exists, its just hidden away somewhere in the display data structures somewhere. Then I remembered that we have code that finds the ColonyPanel in the display data structures. Adapting that allowed me to drop all the instance variable creation and just use the existing instance, and improve matters further by bringing it to the foreground. So, with git.f39127f I think we are done here.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
That should be easy to fix, provided that we can agree on which panels might logically be displayed more than once. In the distant past, most (all?) panels were singletons, but this was abandoned for the sake of simplicity and better localization.
Hey, I'll go ahead and take a wack at this. So far my goal is to transform the panels opened by F-keys into singletons, hope that's fine.
I believe I was able to solve this. I turned all the classes extending ReportPanel and turned them into a Singleton Pattern Design so that only one panel of that instance can be shown at a time.
I looked at the patch, but it is a bit clumsy.
I would have just written:
and BTW, what is the point of the doubled semi-colons? There are several instances.
Lots of duplicate code there in the Report*Panels. Can it be pushed up the class hierarchy? Or do we need a SingletonReportPanel class between them and ReportPanel?
Please do not use tabs. They are really annoying unless everyone has exactly the same indentation conventions and editor settings.
Speaking of which, please be a more vigilant about following the standard Java coding conventions. They do make life easier in the long run.
And yes, I realize the current FreeCol codebase is not exactly perfect there:-).
Wow, I can't believe my clumsy hands actually pressed two semicolons on so many statements.
On another note, I'm not sure quite sure how I should go about pushing all ReportPanels into a SingletonReportPanel hierarchy as I am not that good at design. I'll have to leave each class as separate entities for now.
Okay, I think I was able to solve all of the issues stated save for the hierarchy/reuse code problem. Thanks for the feedback Mike.
I looked over it, but it still bugged me that we have to create all these bogus "instance" variables --- after all, if we are displaying the panel an instance already exists, its just hidden away somewhere in the display data structures somewhere. Then I remembered that we have code that finds the ColonyPanel in the display data structures. Adapting that allowed me to drop all the instance variable creation and just use the existing instance, and improve matters further by bringing it to the foreground. So, with git.f39127f I think we are done here.