Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #2085 path equality overrides #2089

Merged
merged 1 commit into from
Sep 12, 2017

Conversation

Dunbaratu
Copy link
Member

@Dunbaratu Dunbaratu commented Aug 15, 2017

Fixes #2085

There were a couple of things causing this:

1 - While GlobalPath.Equals() has the right logic in it to test equality, PathValue.Equals(), in the PathValue class which is a wrapper around GlobalPath, wasn't using it.

2 - Even with Equals() properly defined, it still wasn't getting invoked because kOS only uses the == operator for equality. (i.e. the kerboscript definition of == for comparing two Structures always has to match the C# definition under the hood for it.) This is because of how CalculatorStructure.Equal() invokes the method op_Equality (via reflection) to do the test.

The code changes here implement those operators for Path.

Here is a more complete version of the tests mentioned in the orginal issue #2085 . When running it, just check to make sure the expected value printed matches the actual result after the colon:

// In order for consructor PATH("somestring") not to crash the script,
// I have to ensure that somestring is a valid path on the volume:
if not exists("1:/file1")
  log "test file 1" to "1:/file1".
if not exists("1:/file2")
  log "test file 2" to "1:/file2".
if not exists("1:/file3")
  log "test file 3" to "1:/file3".

LOCAL pathA IS PATH("1:/file1").
LOCAL pathB IS PATH("1:/file1").
LOCAL pathC IS pathA.
LOCAL pathD IS PATH("1:/file2"). // path which should differ from the rest.

// Tests which should all be TRUE:
PRINT "T1. Expect True: " + (pathA = pathA).
PRINT "T2. Expect True: " + (pathB = pathB).
PRINT "T3. Expect True: " + (pathA = pathC).
PRINT "T4. Expect True: " + (pathA = pathB).
PRINT "T5. Expect True: " + (pathB = pathC).
PRINT "T6. Expect True: " + (pathA <> pathD).
PRINT "T7. Expect True: " + (pathB <> pathD).
PRINT "T8. Expect True: " + (pathC <> pathD).
PRINT "T9. Expect True: " + (pathA:TOSTRING = pathB:TOSTRING).

// Same, but reversed left/right operands:
PRINT "TR1. Expect True: " + (pathB = pathA).
PRINT "TR2. Expect True: " + (pathB = pathB).
PRINT "TR3. Expect True: " + (pathC = pathA).
PRINT "TR4. Expect True: " + (pathB = pathA).
PRINT "TR5. Expect True: " + (pathC = pathB).
PRINT "TR6. Expect True: " + (pathD <> pathA).
PRINT "TR7. Expect True: " + (pathD <> pathB).
PRINT "TR8. Expect True: " + (pathD <> pathC).
PRINT "TR9. Expect True: " + (pathB:TOSTRING = pathA:TOSTRING).

// Tests which should all be false:
PRINT "F1. Expect False: " + (pathA <> pathB).
PRINT "F2. Expect False: " + (pathA <> pathC).
PRINT "F3. Expect False: " + (pathA <> pathA).
PRINT "F4. Expect False: " + (pathB <> pathB).
PRINT "F5. Expect False: " + (pathC <> pathC).
PRINT "F6. Expect False: " + (pathD <> pathD).
PRINT "F7. Expect False: " + (pathA = pathD).
PRINT "F8. Expect False: " + (pathB = pathD).
PRINT "F9. Expect False: " + (pathC = pathD).

// Lexicon example setup:
LOCAL pathLex IS LEX(
    pathA, "file1",
    pathD, "file2"
    // file3 is not present on purpose so we can test missing key cases
    ).

// Note that AFTER setting up the Lexicon, we'll change
// the value of pathA to match what pathD had been.
set pathA to PATH("1:/file2").
// Expected result: Now we should have:
//  pathA = 1:/file2
//  pathB = 1:/file1
//  pathC = 1:/file1
//  pathD = 1:/file2

PRINT " L1. Expect Value file1: " + lexValAt(pathLex,PATH("1:/file1")).
PRINT " L2. Expect Value file2: " + lexValAt(pathLex,PATH("1:/file2")).
PRINT " L3. Expect failure:     " + lexValAt(pathLex,PATH("1:/file3")).
PRINT " L4. Expect Value file2: " + lexValAt(pathLex,pathA).
PRINT " L5. Expect Value file1: " + lexValAt(pathLex,pathB).
PRINT " L6. Expect Value file1: " + lexValAt(pathLex,pathC).
PRINT " L7. Expect Value file2: " + lexValAt(pathLex,pathD).
SET pathD to PATH("1:/file3"). // Note, construct new value for testing:
PRINT " L8. Expect Value file2: " + lexValAt(pathLex,pathA).
PRINT " L9. Expect Value file1: " + lexValAt(pathLex,pathB).
PRINT "L10. Expect Value file1: " + lexValAt(pathLex,pathC).
PRINT "L11. Expect failure:     " + lexValAt(pathLex,pathD).

PRINT "--Done--".

// A way to print a lex key without breaking execution if not found:
function lexValAt {
  parameter lexStruct, lexKey.
  if lexStruct:haskey(lexKey)
    return lexStruct[lexKey].
  else
    return "((key not found))".
}

There were a couple of things causing this:

1 - While GlobalPath.Equals() has the right logic in
it to test equality, PathValue.Equals(), in the
PathValue class which is a wrapper around GlobalPath,
wasn't using it.

2 - Even with Equals() properly defined, it still wasn't
getting invoked because kOS only uses the `==` operator
for equality.  (i.e. the kerboscript definition of `==`
for comparing two Structures always has to match the
C# definition under the hood for it.)  This is because
of how `CalculatorStructure.Equal()` invokes the
method `op_Equality` (via reflection) to do the test.

The code changes here implement those operators for Path.

Here is a more complete version of the tests mentioned
in the orginal issue KSP-KOS#2085 . When running it, just
check to make sure the expected value printed
matches the actual result after the colon:

```
// In order for consructor PATH("somestring") not to crash the script,
// I have to ensure that somestring is a valid path on the volume:
if not exists("1:/file1")
  log "test file 1" to "1:/file1".
if not exists("1:/file2")
  log "test file 2" to "1:/file2".
if not exists("1:/file3")
  log "test file 3" to "1:/file3".

LOCAL pathA IS PATH("1:/file1").
LOCAL pathB IS PATH("1:/file1").
LOCAL pathC IS pathA.
LOCAL pathD IS PATH("1:/file2"). // path which should differ from the rest.

// Tests which should all be TRUE:
PRINT "T1. Expect True: " + (pathA = pathA).
PRINT "T2. Expect True: " + (pathB = pathB).
PRINT "T3. Expect True: " + (pathA = pathC).
PRINT "T4. Expect True: " + (pathA = pathB).
PRINT "T5. Expect True: " + (pathB = pathC).
PRINT "T6. Expect True: " + (pathA <> pathD).
PRINT "T7. Expect True: " + (pathB <> pathD).
PRINT "T8. Expect True: " + (pathC <> pathD).
PRINT "TR9. Expect True: " + (pathA:TOSTRING = pathB:TOSTRING).

// Same, but reversed left/right operands:
PRINT "TR1. Expect True: " + (pathB = pathA).
PRINT "TR2. Expect True: " + (pathB = pathB).
PRINT "TR3. Expect True: " + (pathC = pathA).
PRINT "TR4. Expect True: " + (pathB = pathA).
PRINT "TR5. Expect True: " + (pathC = pathB).
PRINT "TR6. Expect True: " + (pathD <> pathA).
PRINT "TR7. Expect True: " + (pathD <> pathB).
PRINT "TR8. Expect True: " + (pathD <> pathC).
PRINT "TR9. Expect True: " + (pathB:TOSTRING = pathA:TOSTRING).

// Tests which should all be false:
PRINT "F1. Expect False: " + (pathA <> pathB).
PRINT "F2. Expect False: " + (pathA <> pathC).
PRINT "F3. Expect False: " + (pathA <> pathA).
PRINT "F4. Expect False: " + (pathB <> pathB).
PRINT "F5. Expect False: " + (pathC <> pathC).
PRINT "F6. Expect False: " + (pathD <> pathD).
PRINT "F7. Expect False: " + (pathA = pathD).
PRINT "F8. Expect False: " + (pathB = pathD).
PRINT "F9. Expect False: " + (pathC = pathD).

// Lexicon example setup:
LOCAL pathLex IS LEX(
    pathA, "file1",
    pathD, "file2"
    // file3 is not present on purpose so we can test missing key cases
    ).

// Note that AFTER setting up the Lexicon, we'll change
// the value of pathA to match what pathD had been.
set pathA to PATH("1:/file2").
// Expected result: Now we should have:
//  pathA = 1:/file2
//  pathB = 1:/file1
//  pathC = 1:/file1
//  pathD = 1:/file2

PRINT " L1. Expect Value file1: " + lexValAt(pathLex,PATH("1:/file1")).
PRINT " L2. Expect Value file2: " + lexValAt(pathLex,PATH("1:/file2")).
PRINT " L3. Expect failure:     " + lexValAt(pathLex,PATH("1:/file3")).
PRINT " L4. Expect Value file2: " + lexValAt(pathLex,pathA).
PRINT " L5. Expect Value file1: " + lexValAt(pathLex,pathB).
PRINT " L6. Expect Value file1: " + lexValAt(pathLex,pathC).
PRINT " L7. Expect Value file2: " + lexValAt(pathLex,pathD).
SET pathD to PATH("1:/file3"). // Note, construct new value for testing:
PRINT " L8. Expect Value file2: " + lexValAt(pathLex,pathA).
PRINT " L9. Expect Value file1: " + lexValAt(pathLex,pathB).
PRINT "L10. Expect Value file1: " + lexValAt(pathLex,pathC).
PRINT "L11. Expect failure:     " + lexValAt(pathLex,pathD).

PRINT "--Done--".

// A way to print a lex key without breaking execution if not found:
function lexValAt {
  parameter lexStruct, lexKey.
  if lexStruct:haskey(lexKey)
    return lexStruct[lexKey].
  else
    return "((key not found))".
}
```
@hvacengi hvacengi self-assigned this Sep 5, 2017
@hvacengi hvacengi merged commit 4c402cf into KSP-KOS:develop Sep 12, 2017
@Dunbaratu Dunbaratu added this to the v1.1.2.0 milestone Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants