WIP: Fixes #3 - Image Gallery #14
5 Participants
Notifications
Due Date
No due date set.
Depends on
#29 Fixes #19 - Extras menu
Cavemanon/SnootGame
Reference: Cavemanon/SnootGame#14
Loading…
x
Reference in New Issue
Block a user
No description provided.
Delete Branch "feature/ISSUE-3_CG_Gallery"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Please let me know if this level is abstraction is acceptable, I'm not sure if writers mess around with the code and how easily usable is has to be.
I've also added some TODOs where assets are needed (buttons and such).
Some more questions/pointers:
WIP: Image Galleryto WIP: Issue #3 Image GalleryNot gonna lie, the code and work looks great so far. Checked it out and its pretty good even in an unfinished stsage. The writers never touch the code base thus this level of abstraction is acceptable.
See quote 3
Seems cool enough, im assuming to unlock something it'd just be as simple as writing
persistent.imagename = True
.Ideally yes as to minimize work needed when implementing new CGs (also gives anons some nice extra artwork that we may throw in).
This is an assumption that can be granted. They're always 1920x1080. Though, for future proofing, it may be wise to have it not be dependent on the size of the CG and be able to figure it out.
If you don't mind writing a bit of documentation for the code (I for one am not familiar with OOP in python) so newfrens can easily pick up the code base to fix and play around with, that'd be great. If you simply don't have the time, I can draft something up to the best of my understanding and you can correct it for accuracy.
The Main Menu is going to need a bit of reworking to get this all in one place. I rather not resize the buttons as the artists will scream about them losing quality, so I think its best to replace Help & About with a screen that just has the word "Extras" on it and from there, Help, About, Gallery, and maybe more stuff can be found.
In other news, i'll get the artists onto the missing assets. Here is a filler for the blank asset I just made, it might be fine enough to be final.
btw, where does "make_button" come from? Is this just a default renpy method that I simply don't know about that can be applied to other objects or am I missing something here?
here's the buttons our artists got
we're going to start using other branches other master for versions of the game as to keep master stable and able to be easily fell back upon. Update this PR accordingly when you can.
Oh, I'm just now seeing this. Thanks for the feedback and assets.
In regards to
Can be done, however I'd liek to point out that if we can follow a convention, ie.: prefix files to be included (or excluded) in the gallery with something like "cg_", then loading them into the gallery can be done without any hassle.
As for documentation and stuff, sure I can do that. Is there a shared folder liek a google drive or something somwhere?
make_button is a part of Gallery, for which a pretty decent quickstart guide can be found here: https://www.renpy.org/doc/html/rooms.html#image-gallery
Cool, didn't realize this was a built in functionality. I'd put documentation as code comments if I were you. Someone picking up the source should just be able to work with it.
As in the file name will have some prefix to it? That sounds like a lot of code refactoring and file name changes that i'd prefer not to do (as to minimize bugs introducted). Anywho, If you think that your system would be better than simply importing the entire cg folder, then go ahead and just write documentation on how to use it. Though, it'd be nice just to not have to deal with editing a file every time a new CG gets introduced that we want in the gallery.
Of course I'd do the refactoring of existing file names, as there is no other way to guarantee backwards compatibility. That said, there are some minor issues with just listing all of the files, namely:
The gallery is divided into n (currently 3) columns, and if the amount of files is not divisible by it, then the grid component crashes. The two solutions I currently have for this are filling the remaining space with invisible images - easy to do but a bit hacky. Or ditching the grid and manually dealing with x and y alignment. A better solution, but it will take more time to implement.
The other issue is that some cgs are way out of the 1920 * 1080 ballpark. In these cases (in all cases actually) I can scale and crop the image, but that might look retarded without the ability to manually override these things (like the cropped up part can just be a piece of the sky for a BIG image then the crowd of students outside the school).
If any of the listed solutions sounds like something preferable/better usable with your current workflow, please let me know. Otherwise I'll just implement the least hacky and most scalable solution.
Can do. Is it alright if I start using type hints and annotations as well? The latter might make it a bit harder for newcomers to pick it up, but it can be a tremendous help with debugging in the longrun.
I don't see too much of a problem with this, I can see how it may be manual and a bit annoying, but it seems to be an easier to maintain (due to low barrier of entry) to work with. Maybe even have the code automatically correct by adding invisable images to fill it out. Easy code to maintain, easy code to write. Yes, its hacky, but dealing with null data isn't something that seems TOO bad.
If you think that the slower solution of manually dealing with X/Y cords is better, then go ahead, but I do think that it'd be harder to maintain due to the possible brittleness of the code (if the code breaks, it'll be a nightmare to fix and may take just as long if something goes really wrong). If you can acconut for that brittle code, then go right ahead.
Yeah I forgot about the panning cgs. I can see the problem now. Do you think that you could make it where, in those cases, they may have a special flag for being displayed with a pan? Seems like it might be a bit frustrating to code, but it'll be close to how the actual game displayed the images. Other than that, maybe using Ren'py's im.Scale and renpy.check_image_attributes in combination to scale the stuff down to fit on screen would be ideal (though, that may cause aspect ratio issues, but we just need it to be "close enough" to be good).
Sure, but please do put the PEP abstracts for both in as to have a "why" for what these are. Personally, having nice comments on what an unfamiliar thing is makes working with code much easier. Attracting amneture talent and manpower is a bit of a goal due to the nature of this project, so making sure they are able to easily learn from the code and understand the code would be ideal. They're simple after reading the abstracts and should be able to easily educate noobies.
https://www.python.org/dev/peps/pep-0484/#abstract
https://www.python.org/dev/peps/pep-3107/#abstract
Anyway, good work as usual. Hopefully a good solution is found in the end.
Updated with the mess of loading all files. This is now very unclean and untidy, didn't have much time.
Also, the gallery needs to be paginated since loading all images at once causes the app to freeze for multiple seconds.
Either I'm retarded, or the ren'py parser does not actually support type hinting. For now I've done a TypeScript kinda deal with comments.
that works too
Added pagination.
@PrincipalSpears Will probably need button designs for prev/next page, and end/start if you guys want that too.
Added unlocking cgs via a global function, with an example on script.rpy:362.
New known issue: Cgs are only shown as unlocked after restarting the game (for some reason).
WIP: Issue #3 Image Galleryto WIP: Fixes #3 - Image Gallerymaybe make the example a comment instead of an in script modification, we did away with the massive "script.rpy".
Any updates on this feature, how close is it to being done?
Yeah, were gonna remove that before submitting the PR for merge/code review.
Extras screen redo is a hard dependency, so I would finish this after that gets merged.
Although currently still only the 1920 * 1080 images are shown correctly, so I'd have to know if the scale n crop method that I've described above is acceptable (for the thumbnails on the paginated gallery page). I have no idea about panning the bigger images when opened, but I'm willing to bet that ren'py has a default way to show them. And then there's the issue of the cgs only unlocking after restaring the application.
Also the unlocks have to be put in one by one, for which I would need a list or something.
All in all, I'd say a few days, but due to work and exams my time is very limited till the 29th. If this is an unacceptable timeframe, feel free to put someone else on the issue.
We got a PR for that open: https://git.snootgame.xyz/Cavemanon/SnootGame/pulls/29
Guess its better to get it out for review than ponder if its good or not, yeah go ahead.
The script has panning in it, maybe looking there and referencing the documentation could be worthwhile
Looking at the code, maybe its an issue of loadGallery not being ran with the addGalleryItem. Due to this, I don't think the game is being updated until loadGallery is ran again (upon a restart)
Don't worry man, we're not really on a big time crunch. Around the 29th would be good and hopefully will hit the Patchy-Patch5 deadline. If it doesn't and it goes over a few days, big whoop, better late than never. Hopefully the UI redesign will be done soon enough so this can be integrated into that and we can get everything pushed and done with.
Do you have any time you'd be free to finish this up? Not really urgent, but it'd be nice to know. If not, that's fine (real life comes before internet meme games), just state what issues are left to clean up and I'll give a go at it.
Minor thing, but for the sake of not having to locate and copy-paste something everywhere a CG is called, why don't you just modify renpy to call your new function every time we use
show
to load a new CG? The function you want is defined inexports.py
, in the renpy folder that is for some reason not present in the repo. Seems to work with a custom-defined print statement well enough.This way, the effort required by anyone else to impl. this should be exactly 0.
Well, not inherently a bad idea, we'd be working with a custom version of renpy at that point and that might cause issues (correct me if im wrong).
also, we'd have to set it up to detect if the show/scene function to test if an image is actually a cg or not, not too bad, but it'll take a little bit of time.
I'm unsure what sort of issues it would cause, given the game ships with whatever version of renpy you bundle it with. It's not really something users need to or are even supposed to install, so modding it shouldn't pose any issues, though I cannot say that authoritatively.
The simplest solution I can think of, given we already have a script that reads through everything in
images/cgs/
would be to just push everything it runs into to an array (python calls them lists?) and store that somewhere accessible, so we can just doif filename in cgslist:
and the rest of the function can follow normally.I don't suspect this project requires much thought about scope given how simple most of it is so far, so dumping it in global probably isn't a bad idea.
@Caliber, No need for fancy stuff. Just do
renpy.seen_image(cg_name)
for checking if the cg has been seen or not - the rest should be very straight forward and requires zero hassle with the script's script.Well fuck, I guess that's what I get for not reading the fucking manual. I should have guessed they had something like this built in when everyone realized they had basic support for image galleries. Thanks for pointing this out.
The problem of determining whether or not it's a CG still stands though. But... that's pretty easily fixable with this, given the other resources we already have.
Sorry for the absence, was busy putting together the sing project lol. I'll finish this up during the weekend, preferably saturday.
As for monkeypatching the lib show func, I'd actually recommend against it. If nobody wants to go through the scripts we still run a macro in some ide, or grep and replace. Though as the saying goes, you can do something by hand in an hour, or spend a day failing to automate it.
Edit: Although, there might be a way. If show is a legit func (I was so far assuming it's markup for the renpy interpreter), I'm sure we can do a HoF that returns it while unlocking a gallery item. Though that might end up with retarded syntax. I'll see.
The manual is terrible to begin with.
For the most part all of the important CG images are all in one directory, with others key ones are also easy to find. (images/CG, images/animation & images/NotForKids!)
So I went a round with a custom scene/show or monkeypatch. A serious issue is, and this goes for seen_image as well, that displayables and images are pretty different. Calling "scene X with Y" is a shorthand for show, and just calling "scene X" without X having been manually defined does work, BUT it's not loaded to images. Try renpy.list_images, and you won't find "stairs" or "a04" in there, since these aren't manually defined with renpy.image.
A lot of this is just syntactic sugar for the interpreter. The string is just parsed via renpy.displayable, as far as I can tell, and matched with the first file that has a name like that (ie.: spaces and underscores are interchangeable, mostly). Likewise, renpy.get_showing_tags will not list it either, whether loaded by "scene X with Y", or calling renpy.scene(X) and renpy.with_statement(Y) under the hood either.
As such, I'd recommend getting a proof-of-concept proto in the patch that does indeed work, and it can be niceified later on. For now I'll just focus on resolving the conflicts, putting the screen in place (and tidying up its layout), and fixing the issue with differently sized images, preferably with transitions. That said, the current version will only work with the cgs folder automatically, I can add a mnually defineable extension to it, but I'll need a list of such files for that (preferably relative path to game/images/).
accidentally clicked the close btw dont mind me xd
I'll try my hand at getting this working, then.
I am very new to git despite programming for way long, is there some sort of etiquette that I should be following when I make a PR or is it fine for me to just copy-paste shit from other PRs that would obviously be conflicting, for the sake of making it work, or is there some way to make a PR for a PR? I will have to steal what's present in this one since it's all ancillary to anything I would change.
You can just remote add & pull @dagoddamnlazysnake 's branch - work on that and then either PR into his branch (requires waiting for him to accept the PR & merge naturally) or just make a new PR
@Caliber @nutbuster Was this issue handed to someone in my absence? I'm fine with that, took my time afterall, however I'm pretty clsoe to finishing it up now. Only the panning and actual unlocking should remain.
@dagoddamnlazysnake The way you worded your whole last comment sounded like you meant for me to try making a functional version of what I was bitching about with the automatically-marking-images-as-seen thing. Was that not what you intended?
@Caliber I'm sorry for any confusion, I meant to address in general whoever is responsible for making this decision in regards to the patch and milestones.
Edit: Though I don't see these as musually exclusive. I can dish out a prototype to keep the deadline, and someone else can make a easier to use version of it.
@dagoddamnlazysnake it wouldn't be a bad idea to have two heads on it at once, especially given you seem to know more about renpy specifically than I do. I will share what I have here rather than make a whole new commit or anything official and see where it goes from there.
Added basic ass panning for gallery items, and "better" unlocking. I say that in quotes because for the life of me I cannot figure out why it won't unlock the image without restarting the application.
In any case, all that's left is putting in the unlock for each cg, then I'm done unless there are more requests.
WIP: Fixes #3 - Image Galleryto Fixes #3 - Image GalleryRequesting review.
Alright, managed to get a proof of concept up and running. That was a lot more clusterfuck-y than I thought it would be, though only because renpy is fucking braindead in how it handles scope. I assume it's for security, but between the dogshit documentation I tried to skim through and the relative obscurity of the engine itself, I had basically no luck figuring out why rpy scripts and the game's regular assets somehow have different global scopes.
This solution is hacky as fuck but I could not find a better way. It does work 100% though.
The long and short of it is that there's at least one properly global variable accessible to both the virtual rpy scope and the regular outside python world. I decided to just shove everything into
config
for the time being, because evidently that exists in both scopes somehow, despite everything else not. I am mostly experienced with PHP and JS so maybe this is just some python-specific thing I have yet to learn but this seems fucking retarded to my brain. I would have had this done in like 2 seconds if it were not for the autism of whoever created renpy.The only base-game file changed was
exports.py
inside thegame
folder as mentioned above, inside theshow
function, and all that was added was this. This can go anywhere inside the function before the return statement(s)Here's all the changes run through meld for visibility.
Actual code is attached.I cannot attach files.sorry about my absence yesterday, my little bro's computer had a stroke and died.
Reviewing now and game doesn't start from SDK's GUI. Nor does it compile from SDK gui. (seems to be a path issue, check traceback I attached)
Starting from the terminal in the project folder works, but not having the SDK to be able to start is a pretty big problem for quality of life (i.e if people wanna mod the game, they'd have to use the terminal, winfrens suffer).
Also, going through chapter one, there were some small preformance hiccups when a cg was triggered. I was skipping through everything though, so this issue might not be everywhere. Everyone should check this out on their own
gallery itself still is missing thumbnails to each of the image until restart.
thumbnails are stretched, maybe it is just wise to crop them instead. Hell, it might be worth it to just manually crop this stuff for the gallery. I regret not saying to do that before lmao.
scroll bar is still massive.
Panning works! Hurrah!
Images are in a bit of a non-sequiental order. Might be worthwhile for them to fill automatically (if you wish to do that)
So far this is all I can see, basically everything else works good.
I'll check on these, unfortunately can only do so in the evening (family matter - around 10 hours from this post). I'll add some insight on each concern, so somebody can try and fix them if need be.
Compile from the SDK: Could you confirm which part of the path is incorrect on your end? I'm guessing the manual addition of /game/, not sure if that's there runtime. Though that needs to be there designtime, so maybe an isDEV flag for it? My other guess would be something with the slashes, some OS might require backslashes.
Hiccups with unlocking cgs: This could be due to having to reload the entire gallery for this stuff to take effect. Haven't noticed on my end, but I'm not running a potato setup so it can definitely do that on lower end machines. Adding to
persistene.cggallery
is easy, and so is adding togalleryItems
since these are both arrays essentially. The problem isg:Gallery()
, since you can only set up a button/image once, and you can only do them in sequential order. What I'd try first is changing the static unlock from"True"
and"False"
to an actual expression inpersistent
, then changing that instead of callingloadGallery
fromunlockCg
. It might also fix having to reset the game for the img to show. Though it still might need Caliber's config hack to work.Manual overrides for crop and ordering: Can be done more or less easily, just adding a big ol ifelse or switch to
cg
and manipulating the displayable. Ordering them however would require a list in which to order them. I'm sorry but I have so little freetime that it would take days for me to go through it all. In any case, for this the best solution seems to be adding asortGalleryItems
function that's called at the end of load (or after load), and sortsgalleryItems
. Its items are identifiable by filename.Scrollbar: Yeah, that can be removed. I put it there for testing purposes and forgot to remove. Just delete the `scroll="viewport" bit from the screen definition.
Fixes #3 - Image Galleryto WIP: Fixes #3 - Image GalleryOSError: [Errno 2] No such file or directory: '/home/joybuke/Documents/ComputerScience/Projects/Snoot/renpy-7.4.2-sdk/game/images/cgs/'
this was in the traceback
Yeah just manually crop and do an if else, nothing complex but maintainable.
If you wanna do the "sortGalleryItems", go for it, its not urgent to have it all sorted, but it'd be nice.
Still couldn't figure out having both not hiccupy unlock and it not requiring restart.
I looked through renpy's docs again but I can't find anything to just wipe and regenerate a screen. Do you think it would be possible to do some weird hack solution by defining the coordinates with the generator script and then using
show
andhide
or whatever the UI equivalent is on menu open so you can change whatever it references and shoves into whatever coordinates, given you can't just find some way to force reflow every time you open it?might be able to hop in and assist with this since i got my other channel of patch5 work all finished (except final review). let me know if i can help with this - i'll try to reread the thread when im not at work and get a bit more up to speed.
this issue is pretty much done - any objections to closing it
aye, this still breaks renpy's gui compiler and loader. traceback is still the same. Someone should put a check in here to see if you're using the gui version to prevent a break, seems like an easy work around.
Closed because of #55 was merged
Pull request closed