Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simulatior enhancement #54

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

ravirnjn88
Copy link
Contributor

Added title and status bar to the simulator.
Snapshot : https://www.dropbox.com/s/j2va2xiftyx0v0f/TeplateSimulator.jpg?dl=0

@martinrotter
Copy link
Contributor

The statusbar is perfectly fine, but title bar cannot be accepted. You used fixed images as titlebars, with this approach anyone who will write new template would have to create new image for titlebar, which is not acceptable.

You can use widget approach for titlebar. You simply add one more composite widget (two labels placed next to each other horizontally) to simulator window, place and size it appropriately, and displaying template simulation content below it.

The statusbar is perfect, I would code it almost exactly the way you did it. Just try to think more about titlebars. If you feel you cannot do widget-like approach, then separate those two features and propose pull request only with statusbar. :)

@croozeus
Copy link
Member

croozeus commented Mar 8, 2015

I have the same views about the title bar. Let's have title bar as a separate widget as @martinrotter suggested and not as images.

@ravirnjn88
Copy link
Contributor Author

If we use a separate widget approach then still we have to place the game icon in one label. So instead of using the icon.png file why not use the full titlebar image.png file.

@martinrotter
Copy link
Contributor

Because template developers would have to design titlebars in exactly same ways. This is not acceptable prerequisite. It could be expected that template developer creates icon for template, but we cannot ask them to create unified titlebars. This is just nonsence, even if it is quite easy to implement this cleanly.

@ravirnjn88
Copy link
Contributor Author

Got it..Thanks

@ravirnjn88
Copy link
Contributor Author

@martinrotter Changed to separate widget approach as suggested by you. Please review the PR

@ravirnjn88
Copy link
Contributor Author

Sorry for the late reply..travelling for the last one day..:)
I think we should not clutter the formsimulator class with titlebar code. Title bar was the part of game simulation and it should be included in the particular game simulation class.

@martinrotter
Copy link
Contributor

Title bar is "shared" across all templates and it has the same exact looking for all templates. The only thing that changes is icon and the title text. All this attributes make me think for 100% that title bar code should be coded just once, not separately in each template.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants