- User Since
- Aug 18 2018, 10:57 PM (35 w, 5 d)
Tue, Apr 23
Sun, Apr 14
Mon, Apr 8
Sun, Apr 7
Fixed all but the loop logic.
Cleanup & made it a little more pretty.
Sat, Apr 6
Fri, Apr 5
Minor fixes from comments (will do bigger ones later) + gamesetup
Not committed but D1703 uses the three of them.
Furthermore it would help a lot when there was a simple test case, which uses this object already, something like the gamesetup options panel or so
Will try (or you could try the map browser diff D1703 which would show all the possibilities).
Thu, Apr 4
This diff does two things: reduce the size of the memory allocated needed for the array and exploit the 16 bit type values to to check the binary information of upto 16 tiles in one go.
Would improve generated maps times by 3~4x times for all sizes if combined with the other diff of TileClass.
Fri, Mar 29
What uses the underscore?
None but given that the state can only be written with uppercase it might serve in the future as a word separator given that it doesn't conflict with the naming of the methods (but anyway I can just revert this change for now)
Defaults are an anti-pattern, because
(1) it means the caller omits mentioning the value, so when someone searches for the value, he won't find all matches
(2) the behavior of the default value might go unnoticed by the caller (assuming that default always fits because that's why it's the default) but mentioning the value makes the reader more critical as to whether the value is correct, and informs the reader directly which value is used.
(3) is all calls explicitly state the value, then it allows comparison between the calls, and in some cases one might find a better value than the default
That argument is mostly for function argument defaults function x(a, b = 123), I didn't investigate what you did with this default here, but I expect it will match the same pattern.
The default allows to ignore all the events that one might not want to process but still happen (e.g. if in a state "PLACING" you only want to process "onmousemotion" event and ignore all the other types of events) otherwise the FSM will warn you every time you try to process an event in that state that is not defined. Mostly the default method should be an empty function in this FSM case (unless we are talking about those two "default" that I had to define at "PRESELECTED" and "SELECTING.BOX" states given that I could not find a way to refactor them to non-defaults methods.
All in all, these possible unknown "default" calls that might happen can be easily seen uncommenting a warning in the FSM.js file to see what is being called at each processed event.
Thu, Mar 28
For now this patch moves:
- the two big switch that evaluate the states (old fsm)
Wed, Mar 27
Further cleanup, streamlined fsm states logic more.
Mar 27 2019
Reupload (the formatter modified some unrelated lines)
- Integrated all the ACTION_FOO into the FSM
- Removed duplicated code
- Moved most implementations specific methods to their corresponding files
- Created corresponding functions calls to place/preview buildings/walls
- Refactored double/triple click units selection
Mar 26 2019
- Added minor helper functions
- Removed duplicated code
- Made easier to add new input events
- Reorganized input states to a more logical naming/structure
Mar 25 2019
Mar 24 2019
Mar 19 2019
Mar 18 2019
- Sent SDL codes to mod/gui/common (was a TODO).
- Grouped related global variables and functions into an object.
- Replaces var variables for let's.
- Sent GUI events to input_events.js
Mar 17 2019
Mar 16 2019
Improved readability and code clarity.
Well my idea was to show this to test waters and see what people think of it, then I could refractor the rest if all is ok.
Mar 6 2019
Still, it doesn't solve the problem of bloating the base object if we want to make a JS interface for each gui type.
Not sure. But does look deceptively easy.
Technically yes but I'm not sure it won't break something else (like the charts).
The thing is I did look into to changing CText CTextOwnder and similar files but the current inheritance model that is made with is quite difficult to expand so I went for the most straight forward/short solution. Maybe composition over inheritance would give more leverage but that means a rewrite of the GUI.
So can we have middle ground? The new method can be polished and use it at face value for the meanwhile so the chat doesn't hog the lobby.
All chat features will still be the same (needs that new extra tag to follow text bottom given that CList doesn't have it).
Nothing will change for the end user except 0ad being a little bit more performant. Not bad deal IMHO.
Mar 4 2019
That means there is a textSmashup = chatHistory.join("\n") prior to that code, i.e. that the only thing you measure is the caption assignment?
Yes. I'm 100% sure c++ code is the problem.
So it takes 5 milliseconds for .caption = text if text is 500 lines long and 5ms equate to 1/200th of a second.
That is JSI_IGUIObject::setProperty which calls ScriptInterface::FromJSVal and GUI<CStrW>::SetSetting, the latter calls CText::SetupText(), which calls GetGUI()->GenerateText and computes the scrollbar properties.
So it's the GenerateText call on a 500line string that consumes 5ms?
But reading CList::AddItem, that also calls CList::SetupText which calls GenerateText for every item, but your diff replaces that call to a call to SetupTextLastItem removing all GenerateText calls except the one for the last item.
If there were numbers to back that up that would demonstrate that claim.
I did a chart in the previous demonstrating that is indeed GenerateText (what else could be? is clear as water that that for loop in GenerateText is unnecessary to add only another item-.-)
For instance one could call GenerateText to a 500 line string and see how much time that costs.
Already done in the chart.
I guess it's the parsing of the GUI tags that might take so long (linear complexity).
Parsing + text sprite generation
So if the analysis is correct, the SetupTextLastItem part of the diff might be correct to fix that removed TODO in CList.cpp (should just verify that one really doesnt need to rebuild texts or whatever else is rebuilt upon AddItem).
Now the remaining quesiton would be whether the same performance improvement could be achieved for CText and CInput.
Yes it can be done.
Perhaps they could store a vector of GeneratedTexts to implement an AddCaption with time complexity that has less than linear relation to the total caption length;
and then some day enable GUITag parsing for read-only CInputs, and thus make the chat both selectable and high-performant / O(1).
They already do have a vector, just needs someone to implement the JSinterface + methods for each case. :)
Mar 3 2019
@elexis chat from the lobby for 2.5 h at sunday.
Feb 26 2019
I must say i haven't tried that (will try) but I'm 99% sure the problem is C++ given it needlessly deletes all computations as text parsing text sprite creation (expensive) and makes new ones for something which was already done.
Feb 24 2019
Feb 23 2019
Will it get commited or do I need to change/do something? I made the change that broke it so I feel responsible :S
@vladislavbelov is this ok?
Remove setting follow_last_if_visible as is orthogonal to the patch (will add in another patch if this goes through).
There is a important difference between setTimeout and setInterval (at least in Node and web browsers in general)
This debounce thingy looks very nice, could be used in many places in gui.
A little hard to add features without adding complexity. (can be refactored later tho but that's not the scope of this patch)
What is the performance critical part where FPS start to be dropped?
Having the lobby open for 1h (lots of comments on chat)
Is it the JS string building from the message history array or the passing of a long string?
Creates a big string from array.join("/n)
I.e. is Engine.GetGuiObjectByName("chat").text += newline similarly quick as the proposed diff?
First: += doesn't work as there is a JS <-> C++ conversion going on.
Second: even if it worked the C++ implementation would anyways delete all the list and make a new one.
Notice the session chat is not affected by this patch and it has to be rebuilt in case someone selects a different playerview or triggers the diplomacy colors feature (or muted/unmuted someone, or on other events).
Yep. Nothing can be done there (unless if <objects type="list"> is used for each filter but that sounds ugly.
I guess the lobby dialog UI performance is relevant FPS-wise because the lobby can be opened in-game.
Is not about the opening (first load) is about the accumulated text being added as if it were a whole new text each time.
Shouldn't selecting multiple lines of text become possible eventually? Would CList allow for that?
For the moment selected setting tag is a int type so no (an array of ints or a set could).
Feb 17 2019
Fixed some object references annoyance. Made sure shared animation settings don't get modified.
Feb 13 2019
Feb 11 2019
Maybe instead of having a new icon for each bonus and penalty could be sprite combination of icon + bonus or icon + penalty combined? (would also make it easier to add new ones with less effort on the art side)
Feb 10 2019
@elexis I think is pretty much done now. Care to confirm? Thanks.
Feb 9 2019
Feb 8 2019
@elexis changed code to reflect your comments and will refractor GameMap function if you think the rest of the code looks ok.
How about this?
Feb 7 2019
I can also feel the clouds move too fast relative to the city+river+desert+near mountain (looks like if the clouds are very close).
Now can modify text color, object sprite color type and size.
Made coder more clear added some checks, functionality.
Added search function, made code cleaner and made fixes mentioned by @elexis . Still not sure if we should show biomes previews.
Jan 27 2019
Back to how it was.
Add the offset of 5.