mer-curious Posted October 12, 2022 Posted October 12, 2022 Hello Tux! Thank you so much to take the time to walk me through this process. I went the bold way and just forked the code to my account and edited the files on-line using the GitHub page, then I created some pull requests. Please check them and add them to your code if you agree with the changes. I made all the changes I suggested above and also changed "NeoCD" to "Neo-Geo CD" and "bios" to "BIOS". I couldn't find the strings for "Raine controls" to edit the "Load game" and "Save game" options there. I looked for "Decrease" in many files, and that is a very unique word in this menu, but couldn't find any instances of that. It looks to be very hidden in the code or I simply couldn't find them using the browser find tool (Ctrl+F). I would be glad if you showed me where it is. Finally, have you thought about renaming the "savegame" folder to "savestates" in order to match the new wording for this function? Or it isn't necessary at all? I was thinking if it would be better to add the new folder name for new installations but also look for save state files in the old "savegame" folder in case a user updates the emulator by just replacing the executable in the folder. Anyway, I'll wait for your comments on that. Thanks a lot again for your willingness to help me with the code.
Tux Posted October 12, 2022 Posted October 12, 2022 (edited) 2 hours ago, mer-curious said: Hello Tux! Thank you so much to take the time to walk me through this process. I went the bold way and just forked the code to my account and edited the files on-line using the GitHub page, then I created some pull requests. Please check them and add them to your code if you agree with the changes. Well it would have been better to have 1 pull request only for all these changes, which would make only 1 commit to merge, but considering the conditions in which you did that, I guess I have no other choice than to accept that... ! 2 hours ago, mer-curious said: I made all the changes I suggested above and also changed "NeoCD" to "Neo-Geo CD" and "bios" to "BIOS". I couldn't find the strings for "Raine controls" to edit the "Load game" and "Save game" options there. There is a magic command for that when the files are on your disk : grep, it goes like this : cd source/sdl/dialogs grep -i "load game" * -> game_options.cpp: TMenu *load = new TMenu(_("Load game"),menu); -> game_options.cpp: { _("Load game"), &my_load }, 2 hours ago, mer-curious said: I looked for "Decrease" in many files, and that is a very unique word in this menu, but couldn't find any instances of that. It looks to be very hidden in the code or I simply couldn't find them using the browser find tool (Ctrl+F). I would be glad if you showed me where it is. This is in the description of a control, so it's in source/sdl/control.c. What do you want it for ? 2 hours ago, mer-curious said: Finally, have you thought about renaming the "savegame" folder to "savestates" in order to match the new wording for this function? Or it isn't necessary at all? I was thinking if it would be better to add the new folder name for new installations but also look for save state files in the old "savegame" folder in case a user updates the emulator by just replacing the executable in the folder. Or just don't change it, I am not going to look into 2 places for the savegames ! 2 hours ago, mer-curious said: Anyway, I'll wait for your comments on that. Thanks a lot again for your willingness to help me with the code. Ok, I'll go looking at this ton of pull requests then... ! Merged all this manually to be able to merge all these commits in just one, I made the mistake of merging the 1st one from the interface, so the repository had a commit that I was forced to overwrite shortly after, but anyway. Also updated the locale files so that all the translations continue to work. I should at least update the po files to merge the new strings, but I forgot everything about these commands, I guess I should investigate one day...! Anyway it's merged. (and congratulations for your impressive determination to have this done !) Edited October 12, 2022 by Tux 1
mer-curious Posted October 12, 2022 Posted October 12, 2022 (edited) 3 hours ago, Tux said: Well it would have been better to have 1 pull request only for all these changes, which would make only 1 commit to merge, but considering the conditions in which you did that, I guess I have no other choice than to accept that... ! Hello Tux! I'm sorry for creating a lot of pull requests. I didn't know how to gather them all in one pull only, so I did what I could to do it quick. Anyway, I'm grateful for your understanding. 3 hours ago, Tux said: This is in the description of a control, so it's in source/sdl/control.c. What do you want it for ? I meant the "Decrease cpu skip" option in "Raine controls". I couldn't find any of those options in the source looking in the cpp files using the browser find tool. I wanted to be able to review the "Load game" and "Save game" there too and possibly other text. Thanks for pointing me the file. I have already pulled a request for the changes there. 3 hours ago, Tux said: Or just don't change it, I am not going to look into 2 places for the savegames ! I'm OK with keeping the same folder name if it would cause problems both for you in the code and for the current users who update by replacing the executable. The change would be mostly for keeping the reference with the new wording adopted for the function. Either way, I think that "savestates" is clearer in terms of what files are there, because the saved game data is in "savedata", therefore in "savegame" we actually find the saved game states, hence the folder being called "savestates". But you can decide however is better in this case. 3 hours ago, Tux said: Anyway it's merged. (and congratulations for your impressive determination to have this done !) Thank you! It was a pleasure to help you on that. I would certainly do more if I understood a thing about coding. By the way, I guess you forgot to merge the changes in the sound options. At least I couldn't notice it in the master files. Curiously this was the only changed file which created a "dezraj-patch 1" branch instead of a standard "patch" branch. Anyway, I deleted it from my repository. Please let me know if you can merge that, otherwise I can recreate a new patch with the changes in this menu. Thank you so much again for your work. Edited October 12, 2022 by mer-curious
Tux Posted October 12, 2022 Posted October 12, 2022 (edited) I don't think it's mandatory to create a branch for a pull request although it's often done... sound_options.cpp ? Well that's what happens with so many PR, I might have missed one. Well I checked your patch-1 branch, it was about gui.cpp, and your main branch doesn't contain any commit from you, so I don't know where this commit is. It's good, I could retrieved your changes from github, you just won't see your name in the from for the commit, but I cited "mer-curious" in the comment, and updated the locale files too. And the reason why I missed is because I merged all this manually because there were too many, I saw they were named as patch-1, patch-2, etc... so I didn't try to check, I just got all these patches, and missed this one because it was named differently. Next time make all your edits in the same branch then make a pull request for this branch alone, it will make things easier, if there is a next time ! Edited October 12, 2022 by Tux 1
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now