- User Since
- Jan 24 2017, 12:54 PM (97 w, 6 d)
Sat, Nov 24
Testing on fedora 28: patch doesn't solve the segfault, with or without the patch both segfault on lobby join. If required I could get access to a fedora 29 machine for testing.
Oct 17 2018
Aug 5 2018
Don't use a common/ global outside common/
Aug 3 2018
Reflect the duplication avoid
Jul 30 2018
fix an elexis' irc comment
Jul 26 2018
complete as in all quotes around I and II are gone => accept
Jul 16 2018
Which clause of art 18.1 applies?
Jul 9 2018
Some digging in the GDPR leads me to two concerns (I am in no position to say if these are fixed everything is ok but would doubt if there are more problems)
Jul 2 2018
Read the code and comments, but as having very limited knowledge about these matter (nor an urgent will to become an expert), can't say anything about the completeness.
The objective is straightforward now, as the timer is reset on every event that the "winning team" changes
If my allies were defeated or declared war on me, that shouldn't make me lose the victory timer.
Certainly agree for lms, but in an allied victory game loosing a team member for me is enough reason to say the "team" is defeated. But as there is a new team meeting the victory conditions, a new timer is set for that team.
Jul 1 2018
The trick here is defining when the counter needs to be reset, to me defeating a player defending a wonder is enough reason to reset the countdown, one can argue differently however. But as the case is specifically named in the commit message Reset counters on playerDefeat requesting verification, further discussion should be in a trac ticket imo.
Jun 28 2018
Jun 25 2018
Checking for completeness is equal to going through all strings manually, so only checked for the Town Phase, I and II cases:
Jun 20 2018
For some reason (couldn't quickly find why) a formation moves extremely slow right after it is created, when reforming (as in setting another shape) the problem is solved.
Jun 18 2018
binaries/data/mods/public/gui/session/unit_actions.js has one too
So in code it technically needs to be playerName, interesesting (out of scope comment)\
Seems to be a pleonasm between research and technologies, but with costs also there => meh => ok
fine with me too
Jun 14 2018
Jun 12 2018
Seems to be the correct English spelling indeed for the strings, in code I presume it should be BackSlash however => patch complete
If it would crash for mods, the mods would receive an error in the current code, thus not caching the file wouldn't change much there.
In the code there are many more cases of colour which strictly speaking should be changed, but whatever
Jun 11 2018
Looks like fixed
Jun 8 2018
Jun 1 2018
certainly (didn't check if more of these cases occur, presumably yes, but that requires reading through all strings => meh)
Have a look at these also: (AI comments can be left out in imo)
Technically speaking of code and animations, the field is just as "constructed" as any other building, but probably sounds better otherwise
(Don't we have some inline in another revision about this?)
I will thrust you, you just removed that one character in that string (wondering why phab isn't highlighting it properly)
Noticed them some times before, thx for fixing them
patch correct and complete, not sure these kind of patches require a review, but whatever
should be committed in A24 development (so wait for an official leave of commit freeze)
May 9 2018
endless support readded in rP21474
May 7 2018
Apr 27 2018
Got lesser and lesser convinced of this patch over time...
Apr 24 2018
Apr 23 2018
default argument also brings some duplicate hardcodings (for the controller case), so thx for the stubbornness
I misread in last comment, patch actually correct. A hotkey string should be added after the release
Apr 22 2018
Don't see an answer to the bonus question
Apr 21 2018
One should consider crowdyness for these stuff, maybe at some point we need to push the bars in the option menu.
Apr 17 2018
Apr 16 2018
Apr 14 2018
Commitment of images has failed ;P
Ever since I upladed my first patch, the montra has been: "No rounding in sim, only in GUI."
Apr 13 2018
New code looks much simpler and less error prone
Apr 8 2018
works as expected
Cannot reproduce the error, but any looks good and correct and doesn't seems to break anything. Agree with the picked option to not depend on that keyword for gui settings.
Apr 6 2018
(you can abandon revisions yourself, if it became deprecated)
Apr 3 2018
Mar 23 2018
Make SDL_HOTKEYPRESS event, so also hotkeys defined in cpp can benefit
Mar 22 2018
The function looks good and seems already useable in a couple of places in the public mod, (from the top of my mind: tab_buttons, options in options menu, settings in gamesetup). It would be great to let those places use the new function while adding the function to he codebase.
Agreeing on the change, testing says front doesn't fall => accept
(will commit after release as we move some translated strings around)
Mar 21 2018
Make a bool in SDL_HOTKEYDOWN
I understand that, and agree such a function could be useful and actually already is (see last comments). So I am only saying that the two proposed changes (function move and new function) should both get an own revision and that the function should be used in a few places already.
Also, I don't intend to change "updateTopPanel" the function; I can see it works, but I don't fully understand it in detail. This patch merely moves the function out of the hard-to-maintain session.js into a new file. This way modders don't have to keep a copy of session.js, they can simply tweak the small top_panel.js file instead.
Didn't ask for real changes (as in executing the code would result in the same), but only a small style fix.
Mar 20 2018
Is this casting correct?
(Still thinking "moving out of range if killed" is a hack as noticed in that old revision meh)
Mar 19 2018
shouldn't updateViewedPlayerDropdown be in the new file too? ( for getBuildString see D1348)
Patch looks almost done, will get in shortly after the release I guess (string freeze and stuff blocks me from committing)
Given that IMO this patch is a regression of the current state (those names can be longer: "Invasion Force" and vercingetorix_ are already of the same longer or equal), so abandoning the revisions, reopen is felt still required
(one could add a test here and there with units without health components)