8000 Fix order of filter parameters is random, #4262 by low-batt · Pull Request #4263 · iina/iina · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix order of filter parameters is random, #4262 #4263

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Fix order of filter parameters is random, #4262 #4263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions iina/FilterPresets.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class FilterPreset {
}

var name: String
var params: [String: FilterParameter]
var params: FilterParameters
var paramOrder: [String]?
/** Given an instance, create the corresponding `MPVFilter`. */
var transformer: Transformer
Expand All @@ -31,11 +31,11 @@ class FilterPreset {
}

init(_ name: String,
params: [String: FilterParameter],
params: KeyValuePairs<String, FilterParameter>,
paramOrder: String? = nil,
transformer: @escaping Transformer = FilterPreset.defaultTransformer) {
self.name = name
self.params = params
self.params = FilterParameters(params)
self.paramOrder = paramOrder?.components(separatedBy: ":")
self.transformer = transformer
}
Expand All @@ -57,7 +57,7 @@ class FilterPresetInstance {
}

func value(for name: String) -> FilterParameterValue {
return params[name] ?? preset.params[name]!.defaultValue
return params[name] ?? preset.params.get(name)!.defaultValue
}
}

Expand Down Expand Up @@ -115,6 +115,30 @@ class FilterParameter {
}
}

/// An ordered collection of name to `FilterParameter` pairs.
///
/// This structure preserves the order of filter parameters specified in the `FilterPreset` definitions by using [KeyValuePairs](https://developer.apple.com/documentation/swift/keyvaluepairs)
/// instead of a Swift dictionary to hold the name to `FilterParameter` mapping. The order of the parameters dictates the order
/// of the UI controls displayed to the user. A dictionary **must not** be used as that would cause the UI controls representing the
/// parameters to appear in random order.
struct FilterParameters {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should we create a new struct for this? I think we can directly use KeyValuePairs<String, FilterParameter> for FilterPreset.params. For the .get() method, we can implement in the extension of KeyValuePairs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to make it really clear how filter parameters must be contained to avoid people using dictionaries. I'll go ahead and simplify and use KeyValuePairs inline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not possible to implement the get method as an extension to KeyValuePairs because the Key generic parameter is not defined as being equatable. No matter. The approach I used is totally and completely wrong. I missed that there is already a paramOrder. But is it optional and missing from several presets. I'm closing this PR and opening a new one that uses the existing system for ordering parameters.


private let params: KeyValuePairs<String, FilterParameter>

init(_ params: KeyValuePairs<String, FilterParameter>) {
self.params = params
}

func forEach(_ body: (String, FilterParameter) throws -> Void) rethrows {
try params.forEach(body)
}

func get(_ name: String) -> FilterParameter? {
guard let index = params.firstIndex(where: { $0.0 == name }) else { return nil }
return params[index].value
}
}

/**
The structure to store values of different param types.
*/
Expand Down
8 changes: 4 additions & 4 deletions iina/FilterWindowController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,10 +411,10 @@ class NewFilterSheetViewController: NSViewController, NSTableViewDelegate, NSTab
}
if let paramOrder = preset.paramOrder {
for name in paramOrder {
generateInputs(name, preset.params[name]!)
generateInputs(name, preset.params.get(name)!)
}
} else {
for (name, param) in preset.params {
preset.params.forEach { (name, param) in
generateInputs(name, param)
}
}
Expand Down Expand Up @@ -490,15 +490,15 @@ class NewFilterSheetViewController: NSViewController, NSTableViewDelegate, NSTab
// create instance
let instance = FilterPresetInstance(from: preset)
for (name, control) in currentBindings {
switch preset.params[name]!.type {
switch preset.params.get(name)!.type {
case .text:
instance.params[name] = FilterParameterValue(string: control.stringValue)
case .int:
instance.params[name] = FilterParameterValue(int: Int(control.intValue))
case .float:
instance.params[name] = FilterParameterValue(float: control.floatValue)
case .choose:
instance.params[name] = FilterParameterValue(string: preset.params[name]!.choices[Int(control.intValue)])
instance.params[name] = FilterParameterValue(string: preset.params.get(name)!.choices[Int(control.intValue)])
}
}
// create filter
Expand Down
0