[ServerOptions] convert to Integers when not set to default values #4110
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mossheim
reviewed
Oct 5, 2018
telephon
reviewed
Oct 5, 2018
patrickdupuis
force-pushed
the
topic/memSizeFloat
branch
from
ffeb392 to
85e4783
Compare
|
I believe this is ready for another look @telephon @brianlheim :) |
mossheim
suggested changes
Nov 29, 2018
patrickdupuis
force-pushed
the
topic/memSizeFloat
branch
from
d5ff76a to
6fc1146
Compare
mossheim
approved these changes
Nov 30, 2018
|
I gave this another careful look over to spot any typos. @snappizz you can merge when you get the chance. |
|
@snappizz would you like to take a look at this? if not i think it's ready to merge |
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose and Motivation
This PR fixes
ServerOptions.asOptionsStringby making it check its variables for indenticality(?) rather than equality. Now that Floats in SC contain at least one decimal number, it's not longer valid in some place to provide a Float where an Integer is expected. For example, we can't use2.pow(16)to set the server'smemSizebecause it returns65536.0rather than65536[1]. Also, since1.0 == 1, it isn't sufficient for.asOptionsStringto test for equality when checking its variables against their default integer values. Whenfalse, I decided to cast the variables as Integers and reassign them to themselves. The reassignment part is debatable, I suppose.This PR only modifies server option variables that are powers of 2.This PR also fixes a few default values and simplifies the check for
useSystemClock. I purposefully avoidedmaxLoginsbecause there's some strangeness there that I don't quite understand.Update: As per @brianlheim's suggestion, I've removed all the magic numbers from
asOptionsStringand replaced them with references to values stored inside a classvar IdentityDictoinary,defaultValues. Due to an order of initialization error, I had to add a call toClass.initClassTree(ServerOptions)insideScore.initClass.[1] supernova fails to boot, scsynth will not. The latter is an issue for another PR.
Types of changes
Checklist
Remaining Work