HomeWildfire Games

Correctly handle receiving 0 damage as not receiving any damage.

Description

Correctly handle receiving 0 damage as not receiving any damage.

Do not flag a unit as injured when it receives 0 damage.
Do not flag dead units as injured.
Do not mark units as injured when they are full health and killed at one shot.

Cleanup Reduce, introducing a separate function to handle deaths.

Patch By: Angen
Reviewed By: wraitii
Commented By: Stan

Differential Revision: https://code.wildfiregames.com/D1769

Details

Committed
wraitiiApr 19 2019, 11:13 AM
Reviewer
wraitii
Differential Revision
D1769: Do not mark unit as injured when receives 0 damage
Parents
rP22195: [i18n] Updated POT and PO files.
Branches
Unknown
Tags
Unknown
Build Status
Buildable 7207
Build 11739: Post-Commit BuildJenkins

Event Timeline

elexis added a subscriber: elexis.Apr 19 2019, 8:17 PM

Very informative description by Angen that was used in the commit message, thanks for writing and chosing that.
(I hope you have examined possible edge cases)

/ps/trunk/binaries/data/mods/public/simulation/components/Health.js
175

The purpose of JSdoc is to provide a common syntax, to prevent people from using a different comment form in every commit. So if you use a specific syntax, consider the specifications that define the syntax you're using.

184

space after { before }, I think the linting bot has a rule for that

wraitii added inline comments.Apr 19 2019, 9:02 PM
/ps/trunk/binaries/data/mods/public/simulation/components/Health.js
175

I believe that's actually JSdoc

elexis added a subscriber: Stan.Apr 19 2019, 9:13 PM
elexis added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/components/Health.js
175

I believe the contrary, I mean the double braces, not the @return, not the *.
(Hence I mentioned the possibility of looking it up on the specs)
@Stan proposed the syntax in the revision proposal, perhaps he recalls? (But you say you've seen it before too? Then we should be able to find it online.)

Stan added inline comments.Apr 20 2019, 12:05 AM
/ps/trunk/binaries/data/mods/public/simulation/components/Health.js
175
elexis added inline comments.Apr 20 2019, 9:46 AM
/ps/trunk/binaries/data/mods/public/simulation/components/Health.js
175

So it's "Google JavaScript Style".
So far we have only used JSdoc http://usejsdoc.org/

Stan added inline comments.Apr 20 2019, 12:26 PM
/ps/trunk/binaries/data/mods/public/simulation/components/Health.js
175

Mmmh my last message didn't go through.
It honk the record Syntax from Google is good and in my case (I use vscode but it should also work with other IDEs) it allows me not to wast time console.logging variables in messages to figure out the syntax. Here is a snippet on how the jsdoc looks and it also works with records it's just an old screenshot. It also provides me with extension methods for types such array.some string.replqce etc

In D1776#71949, @Stan wrote:
  • Fix Vulkan warning
  • @returns -> @return
  • A little example in Vscode

elexis raised a concern with this commit.Apr 25 2019, 1:27 PM

Taking the red ink because either the coding conventions should be changed or this new Google syntax style and the practices of not abiding by conventions should change.

/ps/trunk/binaries/data/mods/public/simulation/components/Health.js
175

If you want to optimize code for vscode, then this must be changed in the entire codebase, not only one line of one file.
So either configure the editor or change the coding conventions and the codebase along.
Using a different style per commit makes the comments more confusing to both human and machine readers.

Please test whether vscode parses the correct JSdoc syntax to describe properties of an object, then you can have your editor behave the way you want for the entire codebase, not only this line and future lines changed to Google syntax.

/**
 * Assign the project to an employee.
 * @param {Object} employee - The employee who is responsible for the project.
 * @param {string} employee.name - The name of the employee.
 * @param {string} employee.department - The employee's department.
 */

http://usejsdoc.org/tags-param.html#parameters-with-properties

211

(same)

This commit now has outstanding concerns.Apr 25 2019, 1:27 PM
Stan added inline comments.Apr 25 2019, 1:38 PM
/ps/trunk/binaries/data/mods/public/simulation/components/Health.js
175

Sure. It also works.

Stan added inline comments.Apr 25 2019, 1:45 PM
/ps/trunk/binaries/data/mods/public/simulation/components/Health.js
175

For returns, note that it is annoying but if the convention says so I will abide.

elexis removed an auditor: elexis.Apr 25 2019, 7:34 PM

Actually JSdoc documentation is incomplete, see https://github.com/jsdoc3/jsdoc3.github.com/issues/115
Should still use Google Closure Compiler syntax that is part of JSdoc only exceptionally.

This commit no longer requires audit.Apr 25 2019, 7:34 PM