Economy mode #1314
Economy mode #1314
Conversation
|
I'm not sure if there are any change requests I missed but to me this looks good now. PS: I noticed one issue: The games take longer than calculated by the game frame length (~20% on fast speed). I also found out the reason: Video frame limiting. I also created a solution JonathanSteinbuch/s25client@economyMode...JonathanSteinbuch:timing, by keeping track of these little deltas and calling subsequent frames a little earlier, which works great when the game frame length is greater than the video frame length. When the two frame lengths get close, movements can be a little less smooth than normally, but if you want smooth you can increase the frame limit, that makes sense. Of course for a game frame length lower than the video frame length this method can't keep the time accurate (because we would have to do micro skips basically) but it still is a little closer than before. Because this affects the whole game and not just economy mode I should open a new pull request for the timing issue, right? |
|
I'll do a final review once I got some more time. Or @Flow86 if he is faster. I'm aware of the problem and actually we have to handle it and ideally in either way. So making multiple game steps per video frame should be possible too. For a similar purpose (video frame rate, aka software VSync) I created FrameTimer and FrameLimiter classes. Those should be used for that too, but needs checking if they are generic enough. |
| void EconomyModeHandler::UpdateAmounts() | ||
| { | ||
| // Return if the game is over or we already updated the amounts this game frame | ||
| if(isOver() || gfLastUpdated == GetEvMgr().GetCurrentGF()) |
There was a problem hiding this comment.
do we want to update each GF? or should we also say its calculated every "now and then"? specially for lower end computers these calculations are a heavy load at the moment?
There was a problem hiding this comment.
besides this question everything looks good!
There was a problem hiding this comment.
If you go for the "now and then" approach: I remember that there is a "statistics step". I guess that would be a good occasion.
@Flow86 Is the "heavy load" measured? If so, maybe we can improve that?
BTW: Do our toolchains support OpenMP? That would be a simple way to get some more speed here.
There was a problem hiding this comment.
I looked at this. Indeed, if I open the Economic progress window the framerate (without frame limiting) drops significantly. So I analysed and optimized a bit and even changed it to only update the iwEconomicProgress content at most once every second, because that's certainly enough (the statistics step seemed a bit too far apart).
But it doesn't make a significant difference in the frame rate. Indeed, when you do some perfomance analysis, I think you will see that the only thing for the ingame windows that takes any significant amount of time at all is drawing. And this seems to depend on the number and type of control elements displayed in the window, for example the tool window with tool ordering enabled is even much worse than the economic progress window. So I recommend that if the goal is to reduce the load while displaying ingame windows the Draw routine should be optimized, for example by prerendering/baking the whole windows except elements that change into textures or if possible by using vector buffers instead of directly drawing quads or whatever takes time there. But this is vastly beyond the scope of this pull request.
So tldr: I checked, and no, in comparison with the whole situation of drawing an ingame window these calculations are not a heavy load at all.
PS: But now I see that I broke my own test by not updating everything... oops, brb
There was a problem hiding this comment.
Should be all good now again
There was a problem hiding this comment.
I thought about this again, and to be honest, I don't like these last three "performance optimization" commits I made very much, because of stylistic and clarity reasons, and as I wrote it's probably unnecessary to have them. I regret them and think I will revert them tomorrow. Should also make it easier to review as @Flow86 already said before that it basically looks good.
JonathanSteinbuch
force-pushed
the
economyMode
branch
from
7e8914e
to
9d6b2d8
Compare
|
I apologize for my imperfect handling of @Flow86's question. Just for clarification since I didn't hear from any of you in a while here: Are you waiting for any additional input from me on this point or are you just not finding the time to answer? (which is fine by me, no worry, it's a hobby project after all) Best Regards |
|
@Flow86 Any idea why jenkins didn't run? Travis is trash now, so we have to migrate that to GHA but I'd like to have at least a green from jenkins before merging |
|
@Flamefire Jenkins is currently broken because of this: jenkinsci/docker-plugin#821 - temporarily downgraded JUnit and Jackson-API, so it runs for now. |
|
@Flow86 Any idea what that is? It looks new to me:
|
|
probably a cached broken file. I cleared the cache and rerun the build now |
|
Can you rebase please so the new CI runs? |
|
@Flamefire, I did a merge, which seemd fine to me. If you insist on a rebase let me know. |
please do a rebase, feature branches should be merge-commit free |
JonathanSteinbuch
force-pushed
the
economyMode
branch
2 times, most recently
from
b8393a6
to
e6700e8
Compare
…s much as possible of 7 good types)
…display, different approach towards making whole map visible, renamed game length addon. Thanks for the extensive review, @Flamefire.
…emoved team determining every frame
JonathanSteinbuch
force-pushed
the
economyMode
branch
from
e6700e8
to
47e9ee7
Compare
|
@Flow86, as requested, I pushed a rebase. |
I added an economy mode (a la settlers 3 economy mode, i.e. collect (in teams) as much as possible of 7 good types).
This is a new version of my previous pull request #1312. As suggested by @Flow86 in #1312 (comment)_ I moved the addons changes out to a separate branch (with pull request #1313), upon which this branch builds. For discussion of my implementation of the economy mode I opened this pull request while the other is still running.
To activate the economy mode in game, choos the game objective "Economy Mode". This adds an immaterial game object EconomyModeHandler which keeps tabs on the progress the players and teams make towards collecting the 7 good types. Also the EconomymodeHandler has an event which is triggered after a set time which can be changed in the addon menu. The EconomyModeHandler is added to the GameWorldBase and as such is serialized and deserialized when saving and loading (this is necessary to remember which good types to collect).
Also I added an ingame window (accessible from the ingame main menu) that shows the economic progress the player and his team make. At the end of the game also the amounts of the opposing teams are made visible and the map is revealed.
For playing the economy mode I recommend using a large map, not too short a time frame (I mostly play at 2 hours game length) and playing in teams with human opponents. (The AI can also be played against, but since this is meant as a multiplayer mode I didn't change the AI behaviour to collect specific goods)
Please let me know about any things that you would like to see improved.
Best Regards
Jonathan