>> [Stian:] >> I have fixed several performance issues already. For example, we had a full >> repaint of the >> entire map every time the mouse moved 1px! We probably need better testing >> in order to >> avoid such problems in the future, as such mistakes are easy to make. > > [Michael T. Pope:] > That was probably me earlier this/last year. [...]
No problem: Everyone who actually does something breaks stuff as well. Thanks for working on FreeCol and making it better! :-D The point is that we might need to write more automated tests. Writing GUI tests is quite difficult right now since we lack the supporting code. It would have been great having GUI tests that verify that the number of calls on certain methods (like displayMap) are kept below a threshold. We could even automatically check for changes in rendering of both maps and panels by automatically retrieving an image of the current behavior and compare it to a previously retrieved image. >> Adding such optimizations back in might be unnecessary, though, as ten year >> old hardware >> can render the entire map in HD several times per second. In addition, we >> cannot add the >> original code back in without also converting every JDialog back into a >> JInternalFrame. > > The JDialog problem is my doing, and alas, I have to stand by it. I can > remind you of the last time we discussed it below. TLDR: JDialog is a > pain but an unfixable bug that hangs games inside our fake event loop > is worse. Thanks for the reminder :-) The modal JInternalFrame was created when we had synchronous communication between the server and client, with only one game event handled at the time. This avoided having several modal dialogs at the same time (which produces the game hang). The point of making the dialog modal was to prevent changes to the game state before the question was answered. The problem now is that the server can asynchronously update the client while we are waiting for a response. The question then is why we are still using modal dialogs? What about changing code like this: if (gui.confirm("quitDialog.areYouSure.text", "ok", "cancel")) { Player player = getMyPlayer(); if (player == null) { // If no player, must be already logged out quit(); } else { getConnectController().requestLogout(LogoutReason.QUIT); quit(); } } ...into using a callback: gui.confirm("quitDialog.areYouSure.text", "ok", "cancel", () -> { Player player = getMyPlayer(); if (player == null) { // If no player, must be already logged out quit(); } else { getConnectController().requestLogout(LogoutReason.QUIT); quit(); } }); This simplifies getting the response. The modal part of FreeColDialog will not prevent state altering operations before the player gives an answer -- so why bother? >> But then again, I think 250ms latency while moving the map is acceptable. My >> guess is you want the latency to be lower than 100ms? > > 250ms for a big map change is fine. I was seeing really slow single unit > moves (independent of animation) without any map changes. I just did a > quick test of the current trunk and can not reproduce that ATM. When I > get a bit more time together I will haul the old machine out of the shed > and see if it is still there, but things are looking hopeful. That's great news :-) I was refering to Wintertime's: "Though, if you click on the map to move and its not redrawn immediately cause of the many layers thats already too slow IMO." We need less than 100ms on full redraws in order to make map repositioning feel instantaneous -- but I might have taken him a bit too literally. Best wishes, Stian Grenborgen _______________________________________________ Freecol-developers mailing list Freecol-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freecol-developers