Page MenuHomeWildfire Games

Switch range manager from unsigned integers to a proper data structure
Needs ReviewPublic

Authored by wraitii on Jun 4 2020, 9:40 AM.

Details

Reviewers
Itms
Summary

This is 'the' diff that enables D2667 down the line.

It replaces the various u16 and u32 with a proper data structure, quite similarly to @nani's playerData1b, but with a more generic structure.
It's designed to handle up to 64 bits right now, but the design should be transparently extendible (with some work) to an arbitrarily high number.


(On top of D2784)

Test Plan

Needs to be play tested, serialisation-tested, and most importantly perf-tested because this really relies on smart compilers.

Event Timeline

wraitii created this revision.Jun 4 2020, 9:40 AM
Owners added a subscriber: Restricted Owners Package.Jun 4 2020, 9:40 AM
Vulcan added a comment.Jun 4 2020, 9:42 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1780/display/redirect

Vulcan added a comment.Jun 4 2020, 9:47 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2311/display/redirect

Stan added a subscriber: Stan.Jun 4 2020, 9:58 AM

Makes the code more readable to me, but I can't tell if that's the right way ^^

source/simulation2/components/CCmpRangeManager.cpp
337

Still accurate?

source/simulation2/helpers/EnumArray.h
2

I wonder if something like boost doesn't provide such an array?

wraitii added inline comments.Jun 4 2020, 10:08 AM
source/simulation2/components/CCmpRangeManager.cpp
337

Yup'. We're creating a custom version with ones everywhere so that other code can transparently use this.
TBH it's not a huge amount of memory...

source/simulation2/helpers/EnumArray.h
2

Probably, but I'd rather not depend on boost.

I think ICU actually has a rather similar "EnumSet" class.

wraitii updated this revision to Diff 12664.Jul 14 2020, 10:07 AM
wraitii added a subscriber: Itms.

Rebased.

@Itms I'm taking comments on this one at this time ;)

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1003/display/redirect

Itms added a reviewer: Itms.Jul 14 2020, 10:12 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/simulation2/serialization/IDeserializer.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/serialization/IDeserializer.h
|  34| class·IDeserializer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classIDeserializer{' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/DebugSerializer.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/serialization/DebugSerializer.h
| 120| The line belonging to the following result cannot be printed because it refers to a line that doesn't seem to exist in the given file.
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classISerializer{' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/ISerializer.h
|   1| /*·Copyright·(C)·2011·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2011"

source/simulation2/serialization/ISerializer.h
| 120| class·ISerializer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classISerializer{' is invalid C code. Use --std or --language to configure the language.

source/simulation2/helpers/EnumArray.h
|  33| template<typename·Enum,·size_t·SIZE·=·8>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'template<...' is invalid C code. Use --std or --language to configure the language.

source/simulation2/helpers/Player.h
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2010"

source/simulation2/serialization/SerializeTemplates.h
|  33| template<typename·ELEM>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'template<...' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/IDeserializer.cpp
|   1| /*·Copyright·(C)·2011·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2011"

source/simulation2/components/ICmpRangeManager.h
|  76| class·ICmpRangeManager·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpRangeManager:' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/DebugSerializer.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/tests/test_EnumArray.h
|  33| namespace·std
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespacestd{' is invalid C code. Use --std or --language to configure the language.

source/graphics/tests/test_LOSTexture.h
|  26| class·TestLOSTexture·:·public·CxxTest::TestSuite
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classTestLOSTexture:' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/BinarySerializer.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/serialization/BinarySerializer.h
| 120| 
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classISerializer{' is invalid C code. Use --std or --language to configure the language.

source/simulation2/helpers/Los.h
|  33| 
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'template<...' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2635/display/redirect

wraitii updated this revision to Diff 12996.Aug 1 2020, 6:14 PM

Rebased & fix compilation errors.

I ran a quick profiling on this testmap: https://code.wildfiregames.com/D2770#118223
No noticeable difference.

A quick look at assembly shows that all EnumArray functions have been inlined, with the current settings, which makes me relatively confident that this will be about as fast as svn with the same # of players.

A go at 30 max players showed a significant slowdown however, which isn't entirely unexpected, so we must be cautious.

Vulcan added a comment.Aug 1 2020, 6:43 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/simulation2/tests/test_EnumArray.h
|  33| namespace·std
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespacestd{' is invalid C code. Use --std or --language to configure the language.

source/simulation2/components/ICmpRangeManager.h
|  76| class·ICmpRangeManager·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpRangeManager:' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/DebugSerializer.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/serialization/SerializeTemplates.h
|  33| template<typename·ELEM>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'template<...' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/BinarySerializer.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/serialization/BinarySerializer.h
| 120| 
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classISerializer{' is invalid C code. Use --std or --language to configure the language.

source/simulation2/helpers/Los.h
|  33| 
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'template<...' is invalid C code. Use --std or --language to configure the language.

source/simulation2/helpers/Player.h
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2010"

source/simulation2/serialization/DebugSerializer.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/serialization/DebugSerializer.h
| 120| The line belonging to the following result cannot be printed because it refers to a line that doesn't seem to exist in the given file.
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classISerializer{' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/ISerializer.h
|   1| /*·Copyright·(C)·2011·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2011"

source/simulation2/serialization/ISerializer.h
| 120| class·ISerializer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classISerializer{' is invalid C code. Use --std or --language to configure the language.

source/graphics/tests/test_LOSTexture.h
|  26| class·TestLOSTexture·:·public·CxxTest::TestSuite
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classTestLOSTexture:' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/IDeserializer.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/serialization/IDeserializer.h
|  34| class·IDeserializer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classIDeserializer{' is invalid C code. Use --std or --language to configure the language.

source/simulation2/serialization/IDeserializer.cpp
|   1| /*·Copyright·(C)·2011·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2011"

source/simulation2/helpers/EnumArray.h
|  33| template<typename·Enum,·size_t·SIZE·=·8>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'template<...' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2833/display/redirect

Stan added inline comments.Nov 10 2020, 10:50 AM
source/simulation2/components/CCmpRangeManager.cpp
57

Shouldn't that use Enum<LosMask, MAX_PLAYERS + 1> instead ?
Also why did you remove the inlining?

63

No else branch after return, although I guess it all could be a ternar.

1808

Could be inlined