- User Since
- Aug 18 2018, 10:57 PM (44 w, 2 d)
Wed, Jun 12
Nice, what it will become of this diff now?
Needs too may licences & code too ugly to commit.
Closing as it does not archive the desirable results.
Probably wrong approach.
Tue, Jun 11
Sun, Jun 2
Maybe the FMS should have this behavior included when SetNextState is called inside an state with "enter" call, as it seems that by design and the code comment you've shown, is the behavior the coder wanted to always have in this case?
May 13 2019
Replay: look at the houses for stuck units
Seems all units are causing considerable lag if they try to/want to go to the same position (with time the lag increases even more).
May 11 2019
Regardless of the above, I wonder if this is the first such feature for which we are popping an object at runtime from the global nether. That seems like a different direction from our current GUI items.
May 10 2019
May 8 2019
May 2 2019
Apr 23 2019
Apr 14 2019
Apr 8 2019
Apr 7 2019
Fixed all but the loop logic.
Cleanup & made it a little more pretty.
Apr 6 2019
Apr 5 2019
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).
Apr 4 2019
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.
Mar 29 2019
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.
Mar 28 2019
For now this patch moves:
- the two big switch that evaluate the states (old fsm)
Mar 27 2019
Further cleanup, streamlined fsm states logic more.
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.