Added axises and axises values for CChart.
Details
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 5305 Build 9018: Vulcan Build Jenkins
Event Timeline
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
http://jw:8080/job/phabricator/1475/ for more details.
I don't think so, why? Let's imagine if someone set a text color alpha to 0.0, then no one won't see this text. But it's not a problem of a CText.
Because the same graphs produce different axes labels. How should this be a correct behaviour?
About same graphs: it's fixed in this patch already, and it's not the problem with axes.
I meant, that the bug with two 1 on the left chart isn't the chart issue.
What about using a hours:minutes:seconds time format for the x axis? I think it would be cleaner rather than using only seconds with high values.
Also, scores on y axis should just be integers, no need for decimal values?
I agree, about format, it'd be nice, but how to implement it without hacks? In case we do the chart element independent of the summary, we could show any values there, so X-axis could be meters, seconds, bytes, etc.
About decimal, there is an image, which describes why we needs decimal, it has multiple equal axis numbers.
Else looks good functionality-wise
binaries/data/mods/public/gui/summary/summary.xml | ||
---|---|---|
204–205 | why not `format_x="%.0fs"? |
source/gui/CChart.cpp | ||
---|---|---|
249 | But why do we use format strings? What if someone does strange things (there is no limit on format anywhere) with the passed format string (which does come from JS/XML)? Eg using %n, not that having access to JS and XML wouldn't already be an issue, but why add code that just begs to be abused? |
source/gui/CChart.cpp | ||
---|---|---|
249 | That could become a buffer overflow vulnerability if we have input from untrusted sources, i.e. if people could collaboratively style the format of charts and have format being sent across the network. If we don't trust gui/summary/, but source/, then we should exclusively add support for few well defined formats, like printing a number with a user defined amount of decimals, percent numbers (to get the % character) and maybe ingame-time for events? |
source/gui/CChart.cpp | ||
---|---|---|
249 | It currently is one, however it also is a format string issue (which is why I mentioned %n, there's quite some literature about that in case you are bored). I wouldn't say we don't trust the gui, but I'd rather not add code that relies on us always trusting it when there are alternatives where we don't have to. Always assume that you will get input you don't expect, so handle things to cope with those (even if the result is something that isn't useful, at least it shouldn't be harmful). |
source/gui/CChart.cpp | ||
---|---|---|
223 | Why do you think it's unneeded? |
source/gui/CChart.cpp | ||
---|---|---|
223 |
IMO The first line is unneeded because the code is equally easy to read. IMO The second line is needless because we have that comment at least once already. Someone caring about this has to search the code for all occurrences of the bugged code (not for all occurrences of the comment). |
source/gui/CChart.cpp | ||
---|---|---|
223 | Exactly by this reason I leave it as is, because it's easier to find it here. It will look strange if we would have this comment in many places, but in the one - not. |
I'm agreeing with leper and elexis on the topic of format strings. Why do we have something that can be misused or mistyped very easily, and not something like a choice between predefined formats: "time", "integer", "percentage", etc.?
I mean, if you can write a format string (%0.2f or whatever) without having to lookup a guide or reference to such things first, then good for you! But consider that not everyone can. I certainly can't. And to require our modders, who operate predominately in the world of XML & JS, to lookup a guide on string formatting for c++ if they wish to change how numbers appear... No. This is not something they should have to do.
And the fact it's not validated or checked for correctness before being used...
source/gui/CChart.cpp | ||
---|---|---|
109 | Why do the axes form a box, when just two sides are sufficient? | |
source/gui/CChart.h | ||
81 | This function is only used to draw the axes lines, right? So why is it not part of the DrawAxes function? My point is that it is unclear from the function name what this is used for, and there's no accompanying comment to explain why a line-graph-chart would want or need to draw a strip of triangles. And as this private function is only called in one place in this class, then why not merge it? Neither function is particularly long. | |
84 | AddFormattedValue |
How do you want to set precision, add new types, add dynamic formats?
I mean, if you can write a format string (%0.2f or whatever) without having to lookup a guide or reference to such things first, then good for you! But consider that not everyone can. I certainly can't. And to require our modders, who operate predominately in the world of XML & JS, to lookup a guide on string formatting for c++ if they wish to change how numbers appear... No. This is not something they should have to do.
We already use the same standard for JS. So nothing is new for modders. But you suggest to limit modders who knows about this format. They won't be able to change something. To make the "time", "integer", "percentage", ... list moddable, you need to create additional files and modder should know how to edit it and know the format (still should know).
So why we need so much pain. If we could add a format, add styles like ModernPercentageChart, ModernIntegerChart. It's the easy & familiar way for all modders. It's really flexible.
<?xml version="1.0" encoding="utf-8"?> <setup> <NumberFormat name="percentage" maxDecimalPlaces="2" suffix="%" /> <NumberFormat name="integer" maxDecimalPlaces="0" /> <NumberFormat name="fictionalCurrency" maxDecimalPlaces="2" prefix="₡" /> <NumberFormat name="increasedPrecisionDecimal" maxDecimalPlaces="8" minDecimalPlaces="4" /> </setup>
(And "time" would probably be a format defined on the c++ side.) I'm not asking you to implement this exact thing, it's just an idea of an alternate possibility. Easier for non-programmers to comprehend, and can potentially be used elsewhere in the gui.
I would also think that different datasets would use different number formats for the y axis. For instance, map control/exploration and vegetarianism would use percentage, treasures collected and buildings captured would be integers. It's not a one-fits-all thing. But the chart would have a default y-axis format for when one isn't provided with a dataset.
Just a thought.
Of course, we could leave modder-friendly formats for a later patch, in which case at the very least write some validation checks on the format string, and address the other remarks (by leper and elexis, not just mine).
source/gui/CChart.cpp | ||
---|---|---|
109 | Four sides looks ugly, and it's not clear they are axes and not just sides of a box. Most graphs and charts use two axes (or three if there are two sets of overlaid data with different y scales). If gui designers want a box around the chart then they can add one via the use of <object type="image" sprite="BoxSpriteOfTheirChoiceAndOrDesign"/>. (And it should remain an option to have one or not.) And what does the gui background have to do with the number of chart axes? |
We'll have a lot of hardcoded and not flexible things then. Because now it's the XML attributes. What's the different from styles?
(And "time" would probably be a format defined on the c++ side.) I'm not asking you to implement this exact thing, it's just an idea of an alternate possibility. Easier for non-programmers to comprehend, and can potentially be used elsewhere in the gui.
I agree, it's not the best solution. But I don't want to introduce hardcoded and not flexible parts. I prefer to use simple way (formats), because it's short and flexible, it's easier to change/improve in the future, when someone will create an elegant way to do such things.
I would also think that different datasets would use different number formats for the y axis. For instance, map control/exploration and vegetarianism would use percentage, treasures collected and buildings captured would be integers. It's not a one-fits-all thing. But the chart would have a default y-axis format for when one isn't provided with a dataset.
I thought, that the best way would be a JS callback, but it's not supported.
source/gui/CChart.cpp | ||
---|---|---|
109 | Sides != Axes. There are 2 axes, but 4 sides. First results from Google: AoE II: Starcraft: MATLAB: All of them use 4 sides, but 2 axes. Charts should be visible on any background. So we need 4 sides. |
source/gui/CChart.cpp | ||
---|---|---|
109 | And the point of this revision is to add axes to charts. Not sides. Of which there are, as you pointed out, two. And just to prove that I can use images too, a nice labeled image of x- and y-axes, courtesy of wikipedia: And the number of sides (or axes) does not make a chart any more or less visible. |
source/gui/CChart.cpp | ||
---|---|---|
109 | To add visible axes I add sides. Sides make charts much visible, because you see borders, you could see (not here, but at all) values on both sides if needed. Also all elements in our GUI are symmetric, so I don't want to break it. From wiki too (Chart): |
Does not need to be done in this patch, but what about adding some indicators on both axes showing the current coordinates of the mouse(if the mouse is within the chart)?
Or maybe also add some crosshairs like this:
i think having the y-axis values on right is more convenient or having on both sides
also crosshair very nice (pls make it white thin)
Is this just hold back by the axes/sides discussion?
(Then we maybe can just settle down to call the thing sides?)
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/differential/65/display/redirect
Tested the patch and I like what I see!
Decimal places:
About the axis label format, the score is typically in the tens of thousands, so we don't need the 2 digits after the comma.
It might be easier to read if we had space all 3 digits or print it like 40k. But that can be improved later.
For resources we even have millions, so it can be hard to read if those numbers are hundreds of thousands or millions.
Buildings and unit numbers are also integers, so there should be no numbers after the comma either.
Kill-death-ratio is a good use case for decimal places.
Units: I wonder if the labels should have a unit attached to it, like an m for minutes.
Of course that is now annoying to when we can't pass a printf format due to the buffer overflow possibility and
just concatenating that unit to the number likely won't work for some language, so it's meh/tricky.
binaries/data/mods/mod/gui/gui.rnc | ||
---|---|---|
5–6 | Wouldn't be surprised if it's not supported, but it would be good to verify the formats. | |
source/gui/CChart.cpp | ||
109 | Gentlemen, it should be the XML file deciding how things should be styled, so just add an option to allow both? | |
155 | range based loop? | |
157 | axis_font, axis_format_x/y? I suspect we might want to add other labels later. | |
258 | ++? | |
source/gui/CChart.h | ||
81 | I don't recall saying anything about this function in particular, but it's not bad to have it split IMO. |
Using fixed format types is an improvement over the old way. Whilst having them hardcoded in cpp is not ideal, dehardcoding them is something that can be left for another day in another revision. (As is potentially localising number formatting (which is something we don't do anywhere currently).)
I am still concerned that you're still not drawing axis, but sides. As you put it so eloquently, sides != axis (which was and still is my point). The aim of this revision is to draw axis (it's even stated in the revision title). So please do so. Once again: if we really want a box drawn around the charts, then as both myself and @elexis have pointed out, there is already a preferred way of doing that.
And finally: how easy would it be to add captions to the axis? For instance, for the x-axis a caption that says "Time Elapsed"; and for the y-axis, a caption that changes depending on what's selected ("Percentage of Map Controlled" or "Quantity of Food Gathered" or whatever). This would be something to pass with the datasets. (The x-axis caption is achievable via XML, but the y-axis caption would preferably be rotated counter-clockwise 90 degrees, which isn't.) That said, I'm happy to leave these captions until A24.
binaries/data/mods/mod/gui/gui.rnc | ||
---|---|---|
7 | Watch your indenting please. | |
source/gui/CChart.cpp | ||
109 | Exactly. As I wrote months ago: |
The important part is the labeling. A graph loses half the information if there aren't labels, which is why it was a must-really-have feature for a22, then it wasn't finished, so I was happy to see this patch being updated half a year later after crying a bit more :P
I'd even be ok with neither drawing axes nor sides if that solves the problem of adding code you don't prefer.
The hardcoding of the format is not too handy indeed, but the problem is that allowing someone to pass the sprintf format allows buffer overflows.
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 511| 511| Engine.GetGUIObjectByName("summaryText").caption = | 512| 512| g_GameData.gui.isInGame ? | 513| 513| translate("Current Scores") : | 514| |- g_GameData.gui.isReplay ? | | 514|+ g_GameData.gui.isReplay ? | 515| 515| translate("Scores at the end of the game.") : | 516| 516| g_GameData.gui.disconnected ? | 517| 517| translate("You have been disconnected.") : | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 512| 512| g_GameData.gui.isInGame ? | 513| 513| translate("Current Scores") : | 514| 514| g_GameData.gui.isReplay ? | 515| |- translate("Scores at the end of the game.") : | | 515|+ translate("Scores at the end of the game.") : | 516| 516| g_GameData.gui.disconnected ? | 517| 517| translate("You have been disconnected.") : | 518| 518| !assignedState ? | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 513| 513| translate("Current Scores") : | 514| 514| g_GameData.gui.isReplay ? | 515| 515| translate("Scores at the end of the game.") : | 516| |- g_GameData.gui.disconnected ? | | 516|+ g_GameData.gui.disconnected ? | 517| 517| translate("You have been disconnected.") : | 518| 518| !assignedState ? | 519| 519| translate("You have left the game.") : | | [NORMAL] ESLintBear (indent): | | Expected indentation of 5 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 514| 514| g_GameData.gui.isReplay ? | 515| 515| translate("Scores at the end of the game.") : | 516| 516| g_GameData.gui.disconnected ? | 517| |- translate("You have been disconnected.") : | | 517|+ translate("You have been disconnected.") : | 518| 518| !assignedState ? | 519| 519| translate("You have left the game.") : | 520| 520| assignedState.state == "won" ? | | [NORMAL] ESLintBear (indent): | | Expected indentation of 5 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 515| 515| translate("Scores at the end of the game.") : | 516| 516| g_GameData.gui.disconnected ? | 517| 517| translate("You have been disconnected.") : | 518| |- !assignedState ? | | 518|+ !assignedState ? | 519| 519| translate("You have left the game.") : | 520| 520| assignedState.state == "won" ? | 521| 521| translate("You have won the battle!") : | | [NORMAL] ESLintBear (indent): | | Expected indentation of 6 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 516| 516| g_GameData.gui.disconnected ? | 517| 517| translate("You have been disconnected.") : | 518| 518| !assignedState ? | 519| |- translate("You have left the game.") : | | 519|+ translate("You have left the game.") : | 520| 520| assignedState.state == "won" ? | 521| 521| translate("You have won the battle!") : | 522| 522| assignedState.state == "defeated" ? | | [NORMAL] ESLintBear (indent): | | Expected indentation of 6 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 517| 517| translate("You have been disconnected.") : | 518| 518| !assignedState ? | 519| 519| translate("You have left the game.") : | 520| |- assignedState.state == "won" ? | | 520|+ assignedState.state == "won" ? | 521| 521| translate("You have won the battle!") : | 522| 522| assignedState.state == "defeated" ? | 523| 523| translate("You have been defeated…") : | | [NORMAL] ESLintBear (indent): | | Expected indentation of 7 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 518| 518| !assignedState ? | 519| 519| translate("You have left the game.") : | 520| 520| assignedState.state == "won" ? | 521| |- translate("You have won the battle!") : | | 521|+ translate("You have won the battle!") : | 522| 522| assignedState.state == "defeated" ? | 523| 523| translate("You have been defeated…") : | 524| 524| translate("You have abandoned the game."); | | [NORMAL] ESLintBear (indent): | | Expected indentation of 7 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 519| 519| translate("You have left the game.") : | 520| 520| assignedState.state == "won" ? | 521| 521| translate("You have won the battle!") : | 522| |- assignedState.state == "defeated" ? | | 522|+ assignedState.state == "defeated" ? | 523| 523| translate("You have been defeated…") : | 524| 524| translate("You have abandoned the game."); | 525| 525| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 8 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 520| 520| assignedState.state == "won" ? | 521| 521| translate("You have won the battle!") : | 522| 522| assignedState.state == "defeated" ? | 523| |- translate("You have been defeated…") : | | 523|+ translate("You have been defeated…") : | 524| 524| translate("You have abandoned the game."); | 525| 525| | 526| 526| Engine.GetGUIObjectByName("timeElapsed").caption = sprintf( | | [NORMAL] ESLintBear (indent): | | Expected indentation of 8 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 521| 521| translate("You have won the battle!") : | 522| 522| assignedState.state == "defeated" ? | 523| 523| translate("You have been defeated…") : | 524| |- translate("You have abandoned the game."); | | 524|+ translate("You have abandoned the game."); | 525| 525| | 526| 526| Engine.GetGUIObjectByName("timeElapsed").caption = sprintf( | 527| 527| translate("Game time elapsed: %(time)s"), { | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 1. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 526| 526| Engine.GetGUIObjectByName("timeElapsed").caption = sprintf( | 527| 527| translate("Game time elapsed: %(time)s"), { | 528| 528| "time": timeToString(g_GameData.sim.timeElapsed) | 529| |- }); | | 529|+ }); | 530| 530| | 531| 531| let mapType = g_Settings.MapTypes.find(type => type.Name == g_GameData.sim.mapSettings.mapType); | 532| 532| let mapSize = g_Settings.MapSizes.find(size => size.Tiles == g_GameData.sim.mapSettings.Size || 0);
Link to build: https://jenkins.wildfiregames.com/job/differential/134/display/redirect
Much better, thank you.
Feature wise, I think this is pretty much done.
Code-style wise (and I'm no expert when it comes to c++, so I'm going by the coding conventions) it looks ok.
Just so we have it attached to this revision, link to the screenshot you posted on #0ad-dev: https://i.imgur.com/9bkTjsg.jpg
binaries/data/mods/public/gui/summary/summary.js | ||
---|---|---|
339 | " rather than ' is more commonly used. Wondering if units should be included (ie. "Time elapsed (minutes:seconds)"). Meh, it should be clear enough. |
Thanks Vladislav, the patch looks good and the feature is great!
binaries/data/mods/public/gui/summary/summary.js | ||
---|---|---|
337 | Engine.GetGUIObjectByName("chart[" + number + "]").format = g_ScorePanelsData[category].headings[itemNumber + 1].format || "INTEGER"; | |
339 | and xAxisLabel can be inlined | |
binaries/data/mods/public/gui/summary/summary.xml | ||
204–205 | Typically we have those indices at the end of the property name, right? | |
source/gui/CChart.cpp | ||
155 | (I almost had suggested it again) | |
157 | Maybe not formats, but fonts don't seem too unlikely. |
I do count 3 people wondering about the units however and in scientific presentations we also displays the units too.
But the units are usually not part of the title but of the values, i.e. 3s, 3m 20s, 3h 20m.
So maybe we could add a time format in a follow-up patch. (equally the 100k score format)
binaries/data/mods/public/gui/summary/summary.xml | ||
---|---|---|
204–205 | No, all above are in the same style, i.e. chart[k]TypeLabel. |
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 507| 507| Engine.GetGUIObjectByName("summaryText").caption = | 508| 508| g_GameData.gui.isInGame ? | 509| 509| translate("Current Scores") : | 510| |- g_GameData.gui.isReplay ? | | 510|+ g_GameData.gui.isReplay ? | 511| 511| translate("Scores at the end of the game.") : | 512| 512| g_GameData.gui.disconnected ? | 513| 513| translate("You have been disconnected.") : | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 508| 508| g_GameData.gui.isInGame ? | 509| 509| translate("Current Scores") : | 510| 510| g_GameData.gui.isReplay ? | 511| |- translate("Scores at the end of the game.") : | | 511|+ translate("Scores at the end of the game.") : | 512| 512| g_GameData.gui.disconnected ? | 513| 513| translate("You have been disconnected.") : | 514| 514| !assignedState ? | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 509| 509| translate("Current Scores") : | 510| 510| g_GameData.gui.isReplay ? | 511| 511| translate("Scores at the end of the game.") : | 512| |- g_GameData.gui.disconnected ? | | 512|+ g_GameData.gui.disconnected ? | 513| 513| translate("You have been disconnected.") : | 514| 514| !assignedState ? | 515| 515| translate("You have left the game.") : | | [NORMAL] ESLintBear (indent): | | Expected indentation of 5 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 510| 510| g_GameData.gui.isReplay ? | 511| 511| translate("Scores at the end of the game.") : | 512| 512| g_GameData.gui.disconnected ? | 513| |- translate("You have been disconnected.") : | | 513|+ translate("You have been disconnected.") : | 514| 514| !assignedState ? | 515| 515| translate("You have left the game.") : | 516| 516| assignedState.state == "won" ? | | [NORMAL] ESLintBear (indent): | | Expected indentation of 5 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 511| 511| translate("Scores at the end of the game.") : | 512| 512| g_GameData.gui.disconnected ? | 513| 513| translate("You have been disconnected.") : | 514| |- !assignedState ? | | 514|+ !assignedState ? | 515| 515| translate("You have left the game.") : | 516| 516| assignedState.state == "won" ? | 517| 517| translate("You have won the battle!") : | | [NORMAL] ESLintBear (indent): | | Expected indentation of 6 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 512| 512| g_GameData.gui.disconnected ? | 513| 513| translate("You have been disconnected.") : | 514| 514| !assignedState ? | 515| |- translate("You have left the game.") : | | 515|+ translate("You have left the game.") : | 516| 516| assignedState.state == "won" ? | 517| 517| translate("You have won the battle!") : | 518| 518| assignedState.state == "defeated" ? | | [NORMAL] ESLintBear (indent): | | Expected indentation of 6 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 513| 513| translate("You have been disconnected.") : | 514| 514| !assignedState ? | 515| 515| translate("You have left the game.") : | 516| |- assignedState.state == "won" ? | | 516|+ assignedState.state == "won" ? | 517| 517| translate("You have won the battle!") : | 518| 518| assignedState.state == "defeated" ? | 519| 519| translate("You have been defeated…") : | | [NORMAL] ESLintBear (indent): | | Expected indentation of 7 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 514| 514| !assignedState ? | 515| 515| translate("You have left the game.") : | 516| 516| assignedState.state == "won" ? | 517| |- translate("You have won the battle!") : | | 517|+ translate("You have won the battle!") : | 518| 518| assignedState.state == "defeated" ? | 519| 519| translate("You have been defeated…") : | 520| 520| translate("You have abandoned the game."); | | [NORMAL] ESLintBear (indent): | | Expected indentation of 7 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 515| 515| translate("You have left the game.") : | 516| 516| assignedState.state == "won" ? | 517| 517| translate("You have won the battle!") : | 518| |- assignedState.state == "defeated" ? | | 518|+ assignedState.state == "defeated" ? | 519| 519| translate("You have been defeated…") : | 520| 520| translate("You have abandoned the game."); | 521| 521| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 8 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 516| 516| assignedState.state == "won" ? | 517| 517| translate("You have won the battle!") : | 518| 518| assignedState.state == "defeated" ? | 519| |- translate("You have been defeated…") : | | 519|+ translate("You have been defeated…") : | 520| 520| translate("You have abandoned the game."); | 521| 521| | 522| 522| Engine.GetGUIObjectByName("timeElapsed").caption = sprintf( | | [NORMAL] ESLintBear (indent): | | Expected indentation of 8 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 517| 517| translate("You have won the battle!") : | 518| 518| assignedState.state == "defeated" ? | 519| 519| translate("You have been defeated…") : | 520| |- translate("You have abandoned the game."); | | 520|+ translate("You have abandoned the game."); | 521| 521| | 522| 522| Engine.GetGUIObjectByName("timeElapsed").caption = sprintf( | 523| 523| translate("Game time elapsed: %(time)s"), { | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 1. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/summary/summary.js | 522| 522| Engine.GetGUIObjectByName("timeElapsed").caption = sprintf( | 523| 523| translate("Game time elapsed: %(time)s"), { | 524| 524| "time": timeToString(g_GameData.sim.timeElapsed) | 525| |- }); | | 525|+ }); | 526| 526| | 527| 527| let mapType = g_Settings.MapTypes.find(type => type.Name == g_GameData.sim.mapSettings.mapType); | 528| 528| let mapSize = g_Settings.MapSizes.find(size => size.Tiles == g_GameData.sim.mapSettings.Size || 0);
Link to build: https://jenkins.wildfiregames.com/job/differential/142/display/redirect
binaries/data/mods/public/gui/summary/summary.js | ||
---|---|---|
335 | ack, if it still works |
binaries/data/mods/public/gui/summary/summary.js | ||
---|---|---|
335 | What do you mean? |
binaries/data/mods/public/gui/summary/summary.js | ||
---|---|---|
335 | Acknowledged, good change, go commit :-) |
binaries/data/mods/public/gui/summary/summary.js | ||
---|---|---|
335 | Will you accept it? |
binaries/data/mods/public/gui/summary/summary.js | ||
---|---|---|
335 | s0600204 did the review, I only added a style remark and some other comments. |