lotgd / core

Core functionality for Legend of the Green Dragon, a text-based RPG game.
GNU Affero General Public License v3.0
152 stars 15 forks source link

Fix/issue 93 #97

Closed Vassyli closed 7 years ago

Vassyli commented 7 years ago

Fixes #93 and adds a adjust a few minorities.

Vassyli commented 7 years ago

Not sure I like this approach though, since this is basically global state for the game, so anyone calling setNow() will affect other gameTime() users. The time is really local to the viewpoint. Can we add a property to the Viewpoint that is gameTime or now and have scenes use that and leave TimeKeeper->gameTime() as an absolute measure? Maybe put in a warning in the comment? :)

Can you think of any use of gameTime besides in scene rendering?

Right now, TimeKeeper() keeps giving a totally different time every time getGameTime() is used which is not what I expect. Furthermore, isNewDay() uses getGameTime() internally, thus making the result of isNewDay() dependent in the exact time it's run in the code. Providing an additional argument in isNewDay() might solve this issue but this defeats the purpose of having a "TimeKeeper" at all. Currently, TimeKeeper is not keeping the time, but measuring it regularly. If you want to another time point you can easily use convertToGameTime().

I see the issue with using setNow() though with changing the global state (I introduced this mainly for testing reasons). How about using a constructor argument instead, thus setting the global state for TimeKeeper in stone for the full game?

austenmc commented 7 years ago

Yes constructor argument makes sense. Let's do that.

Vassyli commented 7 years ago

Updated PR to implement the changes.