GH-34 Allow more flexible slicing

[henrik-dmg]

Background

This PR changes the process of slicing the final canvas into slices to not rely on the size of the supplied screenshots anymore but rather the user specifying a desired number of slices.
It also fixes a couple of other things, like remove a bunch of dead code and QOL updates

Changes

Changes relevant to #34

• added new key numberOfSlices to DeviceData
• added SliceSizeCalculator type and tests for it
• adapted the validate() method in DeviceData to verify a couple things
  • gap width is >= 0
  • number of slices is >= 1
  • resulting slice size is valid

Other changes

• updated swift-argument-parser to 1.3.0
• bumped minimum Swift version to 5.9 and minimum platform to macOS 13
• fixed CI by specifying which macOS version to use and also set the Xcode version before running/building tests
• updated example config files to contain the new required keys
• added inversion: .prefixedNo to two flags that had an inverted name, this makes it easier to reason about them without doing the mental inversion based on the name
• bumped version to 6.0.0 as the new slicing mechanism requires an update to the config files, which is therefore a breaking change
• moved regex parsing from NSRegularExpression to modern Regex
• removed old clearDirectories and outputWholeImage keys from parsing
• added tests for a bunch of things that were previously untested
• updated the benchmark.py script to not rely on any third-party libraries
• remove a lot of dead code and made many things internal that did not need to be public
  • I used periphery to scan for such code

How to Test

• run swift run swiftframe render Example/example.yaml --verbose --manual-validation
• check that the manual validation info contains the new key Number of slices and also that the Output slice size is displayed
• run tests

Closes #34

[Pispa339]

This makes so much more sense now that I'm seeing this :D

[Pispa339]

I know this means the same as the actual condition, but to me it would make sense to have the condition and the expectation's textual description be the exact same

[henrik-dmg]

I think this follows the pattern of NSError, it has localizedFailureReason, localizedDescription and localizedRecoverySuggestion which this kind of mimics a bit 🤔

[Pispa339]

What I mean is that I find it odd that the condition is numberOfSlices > 0 but then the textual description is numberOfSlices value should be >= 1 instead of it being numberOfSlices value should be > 0 :D

[Pispa339]

Why do we do the above check for numberOfSlices > 0 when we do the same again in SliceSizeCalculator.calculateSliceSize? I'd suggest extracting the validation conditions into another static method e.g. validateSliceSizeCalculationInput(templateImageSize: CGSize, numberOfSlices: Int, gapWidth: Int?) -> Result which we could then call from here and from inside SliceSizeCalculator.calculateSliceSize

[Pispa339]

The Result could e.g. be an enum such as

enum Result {
    case valid
    case invalid(error: NSError)
}

 ¯_(ツ)_/¯

[Pispa339]

Or well, now looking at the method itself, the calculation logic would be almost the same as the method itself so idk 😅

[henrik-dmg]

Yeah I see where you're coming from though, I wasn't super happy about this either 😅 using a static value would have to be a bit more complex though as there can be multiple DeviceData in a single configuration file

[Pispa339]

Neat!

[Pispa339]

Wait what this works 🤯 I always thought one can only do this for a method, not a loop

[henrik-dmg]

A defer statement is used for executing code just before transferring program control outside of the scope that the statement appears in.

Honestly I didn't think about it when I wrote it, the compiler didn't complain 😄 but I verified that it's not accidentally using method-scope but the for-loop-scope with this code:

func deferTest() {
    let values = [1, 2, 3, 4]
    for value in values {
        defer { print(value) }
        print("Loop")
    }
}

prints:

Loop
1
Loop
2
Loop
3
Loop
4

[Pispa339]

🤯 the more you know

[Pispa339]

Codewise looks good and the tests instructed in the PR description are all looking good. Kind of tempted to test this with our screenshot pipeline but it feels like a bit too much of effort. We'll find out once we merge this to master anyways 😅