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

GPS text ui: added cadence, temp, grade and units #1461

Merged
merged 4 commits into from
Aug 13, 2023

Conversation

dany123
Copy link
Contributor

@dany123 dany123 commented Jul 25, 2023

I've added the degrees symbol in some texts, it's part of extended ascii so I think it's fine in the file.

Btw, the long model list looked a bit more organised one entry per line (with a comment stating it's corresponding case value), not sure if you're willing to update the rules of the formatting tool for this but I think it would be a good change.

Copy link
Member

@ddennedy ddennedy left a comment

Choose a reason for hiding this comment

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

You should only include these new options based on the MLT filter version. You can use filter.isAtLeastVersion(3)

@ddennedy ddennedy added this to the v23.09 milestone Jul 25, 2023
@dany123
Copy link
Contributor Author

dany123 commented Jul 26, 2023

filter.isAtLeastVersion(3)

For some reason this isn't working, the call is always returning false, do I need to set the version somewhere myself? I'd expect it to read from the .yml where it's been set previously.

I put these in the main setControls call to test out.

        console.log('v0+?' + filter.isAtLeastVersion(0))
        console.log('v1+?' + filter.isAtLeastVersion(1))
        console.log('v2+?' + filter.isAtLeastVersion(2))
        console.log('v3+?' + filter.isAtLeastVersion(3))
        console.log('v4+?' + filter.isAtLeastVersion(4))

and this is the output:

[Debug  ] <setControls> v0+?false
[Debug  ] <setControls> v1+?false
[Debug  ] <setControls> v2+?false
[Debug  ] <setControls> v3+?false
[Debug  ] <setControls> v4+?false

@ddennedy
Copy link
Member

I added your log line to the top of setControls() in gpstext/ui.qml and it works for me:

[Debug ] v2+?true

There are other filters that use function.
The version is read from the MLT installed $prefix/share/mlt-7/qt/filter_gpstext.yml. Maybe Shotcut+MLT is unable to locate your filter_gpstext.yml. It is not super-critical for the core functionality of MLT and Shotcut, but it is consulted for special situations like this change request. Distro packages do not always have the latest, expected version of MLT for the Shotcut version.

@dany123
Copy link
Contributor Author

dany123 commented Jul 26, 2023

The version is read from the MLT installed $prefix/share/mlt-7/qt/filter_gpstext.yml

This was my bad, thanks.
I've still not managed to properly compile Shotcut (only mlt), so I'm just copy pasting stuff around sadly.
Anyway, the check works now as it should.

@dany123
Copy link
Contributor Author

dany123 commented Jul 27, 2023

Quick question: I'll update the GPS graphic filter with grade support and a few minor fixes, do you have a preference if I should add them all in this PR or make a separate one?

@ddennedy
Copy link
Member

This is not going into this weekend’s release, and then I will be on vacation for 2 weeks. So I don’t have a preference, and you can choose to just use this branch.

@dany123 dany123 marked this pull request as draft August 1, 2023 21:52
@dany123 dany123 marked this pull request as ready for review August 1, 2023 23:04
@ddennedy ddennedy merged commit 09a8024 into mltframework:master Aug 13, 2023
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