sloukit / pydew-valley-uzh

Mod of Pydew Valley for an experimental study in Psychology
Creative Commons Zero v1.0 Universal
32 stars 61 forks source link

Fix Bug : inventory and settings menu merge problem #184

Closed ZedKay52 closed 1 month ago

ZedKay52 commented 1 month ago

When you press "i" for inventory and then ESCAPE for the settings the two screens (inventory menu and settings menu) will be merged, this pull request fix this bug by making ESCAPE for exiting the inventory screen Bonus : When solving this problem I added a function in main (check_current_state) that hopefully keeps track of the current game_state (I used it to fix the bug) , maybe it will be useful later !

-- Sorry, I closed the pull request by accident .

I run the formatlint.py script and it worked fine !!

DangerousVanilla commented 1 month ago

Hey @ZedKay52, wondering if you don't mind sanity checking something for me... I'm not seeing this issue in the main branch anymore. I've merged a few other PR's today and I believe one of them fixed this issue without having to remap the key. I'm not 100% sure at the moment but I think it might have to do with the removal of set_token_status which was found to not be serving a purpose. Just wondering if you might pull a fresh copy of main and see if this display issue is resolved for you too?

ZedKay52 commented 1 month ago

I will see

ZedKay52 commented 1 month ago

Yeah, it worked so no need to this pull request By the way, when you said earlier " would you mind bringing this up to date with main". What did you mean ? and how i can actually do it ? (Sorry if that was trivial I am just a beginner) and also, when I run formatlint.py i should mention that it worked ?

---- This PR will close as soon as this comment is replied

DangerousVanilla commented 1 month ago

Good deal, thanks for confirming. As for bringing it up to date with main I'll do my best to explain! GitHub's processes and terminology can definitely get confusing when starting out so no worries there.

Basically, we have the main branch (literally named main). Because we have a bunch of people developing at the same time there are usually multiple PR's open at any given time and may or may not be merged with the main branch at any time depending on the outcome of testing them. So if you pulled your reference branch from main, say, a week or more ago, it's very likely there have been changes to the main branch (via merged PR's) since then meaning when you commit yours you're potentially submitting an outdated copy + your changes which almost always results in conflicts between main and your PR. To avoid this it's good practice to ensure your branch is up to date with main, especially before you commit.

If you happen to be using github desktop it's pretty easy - just select your branch from the middle dropdown labeled 'current branch' at the top, then click the button at the bottom: image

merge_steps

If you're already on your branch in the current branch dropdown you don't have to click it again technically - you'll notice the button at the bottom is dynamic and displays the name of the branch you're targeting.

You'll then be presented with a popup to select the branch to pull from - ensure 'main' is selected and you'll be presented with a warning at the bottom if it detects any conflics, like so: image

Go ahead and click the blue button 'Create a merge commit' - it will force you to fix the conflicts before you're able to actually perform the merge. You'll be presented with the following where it indicates the detected conflict(s) and gives you the chance to resolve them: image

Once they're resolved you should be able to merge main to your branch. It'd be a good idea to test again at this point before committing your PR. And of course don't forget to run the formatter (formatlint.py) as the last thing you do before committing to ensure you don't have a test failure when we're reviewing.

Here's a link to the official documentation if you're interested!

ZedKay52 commented 1 month ago

Thanks !