Add setting to reset "previousTracksExpaned" each time a queue is opened

[HeroBrine1st]

Changes

• Update nix flake and CONTRIBUTING.md to allow contributing from NixOS at least with workarounds
• Add setting to reset "previousTracksExpaned" each time a queue is opened (this behavior is disabled by default but the setting is inversed).
• Add a workflow check for codegen verification

Screenshot

Todo before merging

• Add a workflow check which checks that code generation was done properly; or
• check that change from ..writeByte(137) to ..writeByte(138) is okay.

• Translations

• Reset Settings

• Extended CONTRIBUTING.md

Related Issues

Not related to any issues

[Chaphasilor]

Great, another test that will lead to failed PR run notifications 🙃
/s
Thanks for these changes. The codegen check is probably good, although I wonder if it's really necessary to run this on every commit (then again, it's probably more lightweight than the full builds).

I'll try to slightly revise the working for the setting tomorrow before merging.
Also, does this PR supersede #1490 ?

[HeroBrine1st]

to run this on every commit

I wish there was a way to reuse "images" of actions because, as you can see, I literally copied first job and simply replaced actual check. And it doesn't change much between different revisions

Also, does this PR supersede #1490 ?

Only parts that add dbus and update flake.lock file, the remaining can be merged but there will be conflict because it also updates flake.lock file. My flake.lock is simply newer so theirs¹ can be discarded cleanly.

Footnotes

1. I'm not english native so if it's not appropriate to say it like that please tell me :P I actually recalled "accept yours" "accept theirs" in IDEA conflict resolution window ↩

[Chaphasilor]

Alright. Yeah Docker-like layers to share would be neat. Maybe there's a way set multiple check results from a single action somehow? We won't be able to consolidate most other actions anyway since they run on different arches and OSs, but for the checks it really is more of just the same, and could all be added to the linux build action...

And "theirs" is absolutely fine :)

---

Regarding the PR, why didn't you put the overwrite at https://github.com/herobrine1st/finamp/blob/285ebcc2859d0c2863b4c52e68e42b24d57d568f/lib/components/PlayerScreen/queue_list.dart#L175? That seems like a much cleaner solution.
Additionally, the setting toggles if toggling the queue list is remembered - technically that could include people wanting to always open the previous tracks list, instead of never? So I'm wondering if instead of the boolean setting, we should just have a dropdown for "When the queue panel is opened, previous tracks are..." and then 3 options, "Shown", "Hidden", "Like before"?
And then we always track the previous state, but ignore it depending on the setting?

That might be cleaner, simpler to understand, and more flexible?
But let me know what you think, I'm rather impartial here. I'll change the wording once we've settled on a solution :)

[HeroBrine1st]

Regarding the PR, why didn't you put the overwrite at

Because tunnelvision :-) It could be worse - the first variant used "whenComplete", and I'd send that if I didn't stumble on codegen

I like your thoughts. My use case is just "wow antistress" since it has very satisfying close animation and when it scrolls past previous tracks instead of closing it's hurting.. again, tunnelvision :P but always opening is probably good for.. desktops! At least while there's no dedicated desktop UI

UPD: I probably know why I tunnelvisioned. I didn't see remembering that header is expanded as feature until I opened the code and saw that it is an actual value in settings DTO, and thus a feature, not a bug.

[HeroBrine1st]

So it looks like this

Screenshot

Regarding this:

Regarding the PR, why didn't you put the overwrite at

finamp/lib/components/PlayerScreen/queue_list.dart

Line 175 in 285ebcc

FinampSetters.setPreviousTracksExpaned(expanded);

?

It's not viable here because it is a callback, so overriding the value where it currently is is the most clean as showQueueBottomSheet function is a chokepoint between 4 usages and another function from dependency.

It can also be done here:

finamp/lib/components/PlayerScreen/queue_list.dart

Lines 103 to 118 in fedebf8

	@override
	void initState() {
	super.initState();
	_queueService.getQueueStream().listen((queueInfo) {
	_source = queueInfo?.source;
	_previousTrackCount = queueInfo?.previousTracks.length ?? 0;
	});
	_source = _queueService.getQueue().source;
	_previousTrackCount = _queueService.getQueue().previousTracks.length;
	_contents = <Widget>[];
	widget.scrollController.addListener(_updateJumpToTop);
	}

But then it is violation of responsibility I think, because it changes application state while initState is for Widget state I guess

[Chaphasilor]

I honestly wouldn't mind having it in initState, that seems easier to find to me. But I'm curious what @Komodo5197 thinks about it?

The dropdown looks fine, thanks! I'll change the wording to be a bit less technical.

[Komodo5197]

I feel like initState makes the most sense, because the widget is what actually uses the value, and this is basically just widget state we're persisting. If the queue was embeded in the player screen, as an example, we'd still want this code to run as the queue loads, and initState would do that.