stashapp / CommunityScripts

This is a public repository containing plugin and utility scripts created by the Stash Community.
https://docs.stashapp.cc/add-ons/
GNU Affero General Public License v3.0
190 stars 147 forks source link

Updates to Timestamp Trade, Attempt 2 #428

Closed vamiceDev closed 2 months ago

vamiceDev commented 2 months ago

I'm not that great at Git, a skill that's on my list to learn. So I just made a new repo with the same name and a single commit.

@feederbox826 Thanks for the heads-up.

Updates TimestampTrade to allow the user to add a tag/update the title of created timestamps.

feederbox826 commented 2 months ago

your commits were not under vamiceDev, figured I would give you a heads up. tips for splitting identities

also cc @Tweeticoats

vamiceDev commented 2 months ago

Yes, I saw right after I submitted, I appreciate it.

Tweeticoats commented 2 months ago

runOnScenesWithMarkers is not a flag that I want because it's already covered by the setting createMarkers. The plugin should run on the scenes as there are other functions that the user may want so the user could disable creating markers but still have the plugin to create movies.

I'm not against adding a tag and adding a prefix to markers. For consistency I would use [Timestamp] or similar as the plugin uses [Timestamp: Skip Submit], [Timestamp: Skip Sync], [Timestamp: Auto Gallery] so it would be more consistent.

I'm working on my own pull request so I'll add the changes and submit that instead if that is ok.

vamiceDev commented 2 months ago

runOnScenesWithMarkers is not a flag that I want because it's already covered by the setting createMarkers.

I don't see how. The query run when "processScene" == PLUGIN_ARGS uses "has_markers": "false". Also, there's a check in the processSceneTimestamTrade function that checks if len(s["scene_markers"]) == 0.

The use case I'm targeting here is a user who made a custom marker for a scene and now wants to add extra markers from timestamp.trade. As it's currently written, that scene will be skipped.

For consistency I would use [Timestamp] or similar as the plugin

Sure that's fine.

I'm working on my own pull request so I'll add the changes and submit that instead if that is ok.

Yeah, that's fine, but I would like to see the functionality from runOnScenesWithMarkers added.

Tweeticoats commented 2 months ago

I have multiple tasks:

If a scene has markers it will skip the scene unless you enable overwriting markers. I'm happy to have a merge setting if you would find it useful.

vamiceDev commented 2 months ago

I see my mistake now, missed the Reprocess All hook, so that's fine.

Suggestion: Calling it Re-Process All implies it only runs on previously processed scenes.

Having an option to merge would be preferable, and I'd argue that it should be on by default.

Also, overwriteMarkers is not in the timestampTrade.yml file, so it's not in the Stash UI.