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

Generate performer checksum from name instead of image. Use default performer image #109

Merged
merged 10 commits into from
Oct 24, 2019

Conversation

WithoutPants
Copy link
Collaborator

@WithoutPants WithoutPants commented Aug 21, 2019

Resolves #67

Changes the checksum generation for performers to be based on name instead of image. Name should be unique for performers anyway, so it makes sense to generate the checksum on the performer name. This also means an image can be reused (for generic images).

This change also adds a default generic performer image, for when no image is provided. The current image is really just a placeholder, and I'd like a better one. Ideally, the replacement would be similarly compact.

icons8-female-user-100

@WithoutPants
Copy link
Collaborator Author

Added the same change for studios (I was going to make a separate PR, but it required the same utils method).

Added a placeholder studio image as well.

Resolves #58

Video-03

@echo6ix
Copy link
Contributor

echo6ix commented Aug 21, 2019

Added the same change for studios (I was going to make a separate PR, but it required the same utils method).

Added a placeholder studio image as well.

Resolves #58

Video-03

As discussed on Discord, it might be a good idea to create an option in Settings > Interface Configuration > Scenes to toggle on/off the display of the studio as a text (use the provided studio name) or as an image.

I'm not sure if providing a Studio Name is mandatory when creating a studio. If not it might be a good idea to make it mandatory to avoid outputing a null string if they turn off studio images on the scene pages.

@bnkai
Copy link
Collaborator

bnkai commented Aug 21, 2019

@WithoutPants
Afaik the checksum from the image was also used for the import function (for both performer/studio)
Have a look at the task_import.go file (use of ProcessBase64Image function from utils/image.go to get the checksum).
So there may be side effects to the point of messing up the import.
It might be enough to edit task_import.go and calculate the checksum there also from the json name
I'm not sure if it matters for the export.
Another matter is also the recalculation of the checksum for performers / studios already in the DB so that the calculation method is consistant for all. An export / import sequence probably is enough , provided that import,export is patched as mentioned above.

Better wait for Stash to tell you in detail though

@Leopere
Copy link
Collaborator

Leopere commented Aug 22, 2019

Is this PR a WIP? If it is please add WIP into the PR name so its not possible to merge until you remove it.

@WithoutPants
Copy link
Collaborator Author

Is this PR a WIP? If it is please add WIP into the PR name so its not possible to merge until you remove it.

It wasn't until @bnkai 's find. Fixed up the issues found.

@WithoutPants
Copy link
Collaborator Author

Another matter is also the recalculation of the checksum for performers / studios already in the DB so that the calculation method is consistant for all. An export / import sequence probably is enough , provided that import,export is patched as mentioned above.

It shouldn't really matter if it is inconsistent for existing data. You could indeed correct it with an export and re-import, but otherwise the app is going to function the same regardless, and the checksum is only exposed to the user during the export/import anyway.

@iKeats
Copy link

iKeats commented Aug 23, 2019

Yo folks. I started making a NoID performer image for the default .... I then liked the idea and made 40 of them ahah. Attached a zip with all of them. My idea, and it would be awesome if doable, is to have the system pick one at random for each new entry. This would alleviate the repetitiveness that a single picture would create. Let me know :)

Preview:
NoName04

NoName12

NoName.zip

@Leopere
Copy link
Collaborator

Leopere commented Aug 23, 2019 via email

@WithoutPants
Copy link
Collaborator Author

Added a packr box with the images @iKeats kindly provided. Changed so that a random image is selected from the images if none is provided.

@WithoutPants WithoutPants changed the title Generate performer checksum from name instead of image. Use default performer image [WIP] Generate performer checksum from name instead of image. Use default performer image Aug 25, 2019
@WithoutPants WithoutPants changed the title [WIP] Generate performer checksum from name instead of image. Use default performer image Generate performer checksum from name instead of image. Use default performer image Aug 27, 2019
@WithoutPants
Copy link
Collaborator Author

Merged from master. Ready for retesting.

@bnkai
Copy link
Collaborator

bnkai commented Oct 17, 2019

I really like this PR and i just need to test the import , export ( the other parts were tested ok way back) but given the major change for the checksum should we wait for feedback from stash himself?

@bnkai
Copy link
Collaborator

bnkai commented Oct 23, 2019

Given the lack of feedback i tested this out again. It looks ok

@Leopere
Copy link
Collaborator

Leopere commented Oct 24, 2019

@iKeats you should make these silhouette designs for all of the tags good grief they look fantastic.

@Leopere Leopere added feature Pull requests that add a new feature improvement Something needed tweaking. labels Oct 24, 2019
Copy link
Collaborator

@Leopere Leopere left a comment

Choose a reason for hiding this comment

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

approved

@Leopere Leopere merged commit d7271d7 into stashapp:master Oct 24, 2019
@WithoutPants WithoutPants deleted the optional_performer_image branch February 5, 2020 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull requests that add a new feature improvement Something needed tweaking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saving performers with the same image
5 participants