- User Since
- Dec 23 2016, 12:04 AM (121 w, 1 d)
Sun, Apr 7
Sat, Apr 6
Fri, Apr 5
Thu, Apr 4
Tue, Mar 26
I listed myself, because I thought not listing myself leaves the impression I didn't review the patch. But you are right, committing implies a review by the committer, so listing myself is redundant.
Mar 20 2019
what part don't you get?
Use D844. Fix minor style issues. Use correct style.
Game works for me. (Intel Xeon with Ubuntu Gnome)
My concern was fixed. Thanks.
Mar 19 2019
In general that looks like a good patch. The idea to group variables and functions that touch the same topic into globals sounds ok, but have you thought about using files for grouping?
Regarding the state machine (inputState) I would strongly recommend looking into using a proper fsm implementation with entry and leave functions. (I think we have a fsm.js somewhere) That could also help fixing some bugs like #4848
Also related to #2826
Mar 14 2019
Mar 13 2019
Doesn't sound like a good alternative, as some language might want to e.g. add the symbol at the end of the line...
XML would be an alternative but had the disadvantage that one can't use the same GUI object structure for every tooltip.
JSON would be an alternative, but I don't see where the txt format is worse than JSON for the loading tips.
So yes I think (and it seems like you do too) it is best to modify each line.
Taking my two comments aside that looks pretty good. (Also it was accepted by others already...)
Mar 10 2019
Shouldn't you also add the building to the map "Sandbox - The Gauls"?
I guess you also want to add tests for that new feature :)
Mar 7 2019
Mar 6 2019
Mar 3 2019
The question is whether it's a good idea to assign three actions to buttons in general and thus commit the patch, or wether whether alternate patterns, such as one action per button are more intuitive.
Perhaps there are some fringe examples, I guess the minimap button already is one of the fringiest. So perhaps it's okay to add that support in the engine but advise people to better use a separate button instead for regular buttons, unless they have a really strong use case where it's also a bad idea to use global hotkeys.
Considering the Minimap use case, it's also strange that this event doesn't send the coordinates to the minimap. I guess it works that way: user clicks, code is triggered, code gets the coordinates from the minimap object? I have a revision proposal somewhere to allow sending a JS::Object to the GUI event.
It should really be interchangeable for people without the third mousebutton. Don't OSX hipsters have only one mouse button?
Isn't the ScriptEvent("worldclick", coords); + IsHotkeyPressed("sendping_minimap") enabling that?
That's more or less the same as my first bullet point.
As far as I can see they don't change anything with the enums, except that the MessageTypes are moved into a separate file.
What are you waiting for? ;)
GUI events are for actions executed onto a specific GUI element. Hotkeys are global events or apply to the focused GUI element.
Polling or only reacting if minimap is focused is not suitable, so we have to use GUI events. For those without a middle mouse button, we could implement a gui button. (Analogous to the unit commands which can be triggered either by key or by buttons)
It's already supported, see default.cfg:
pan = MouseMiddle ; Enable scrolling by moving mouse
There it is used as global Hotkey and checked via polling.
Implement suggested changes
Fix the addded comments ^^
Add comments to the Schema
Sorry to ask, but does this need petra support?
It seems like you made the diff dependencies wrong around.
I think this revision should be the parent of GridBrowser GUI addon.
(Everyone gets confused by that at least once ;))
Feb 15 2019
Thank you for the patch.
And sorry, that it took so much time :/
I tend to the same opinion.
But maybe u want duplicate these options in each tab?
Definitely not a good idea ;)
Feb 8 2019
Feb 4 2019
Add a option to disable the experience status bar
Fix license header year
The only question that remains: Should it be enabled by default?
It could be quite annoying, so I'd lean to disable it by default.
What do you think?
Jan 29 2019
Was there since leper implemented Healing in rP11536
Jan 28 2019
Jan 26 2019
Adding an option for this is a good idea.
Will do so as soon as i have time for it.
Jan 23 2019
Jan 18 2019
Support for multiple pings. Use player color for pings. Ping is shown as increasing square. (hope someone will code something more beautiful) Limit each player to 1 ping per second.
That is the minimap attack notification. (white blinking unit)
Use sim command instead of chat. Play sound. (but the wrong) Create ping entity on real map. (but the wrong)
Ok, you convinced me. It'll be a sim command. (Was my first approach, but I thought such only visible thing should be better handled like chat)
Finishing to read the code, I see there is no local entity created yet.
As already stated in the TODOs in the diff description.
IMO that would be very relevant, we have a bunch of flag actors, and perhaps we can find something shiny to make it more visible without trying to find the differently colored pixel on that small thing in the bottom left corner.
The plan is to make more than just a tiny pixel in the minimap. I thought about expanding concentric circles or such.
Cosmetic changes (Year numbers, semi)
If someone had read the linting results of the patch....
(Yeah, I know input.js throws a bunch of warnings)
Jan 17 2019
Jan 16 2019
If nobody objects, I'll commit that soon.