Stardew Valley: implement new game #1455

Jouramie · 2023-02-15T05:17:40Z

What is this fixing or adding?

Add Archipelago implementation for Stardew Valley

How was this tested?

3 play tests with the community, two asyncs and one sync.
A lot of feedback and bug reports
Releases of .apworld published https://github.com/agilbert1412/Archipelago/releases

test/TestBase.py

…in AP core

black-sliver

I started looking at things, here are couple so far:

we can not really require pytest
For two reasons:
1. pycharm has its own thing that (hopefully, some) devs use during their development
2. pytest's collection code does not pickup packed apworlds and pytest does not honour load_tests
The two "offending" things I see in the PR are
1. pytest.mark
2. using "bare function" tests (that live outside of a class derived from unittest.TestCase)
Not sure what I think of test.TestBase.WorldTestBase.player
worlds/sv/data has no python code in it, but you have an __init__.py there.
I believe a pure data folder should not be a python package, so the __init__.py should probably go.
A doc string at the top of scripts/*.py would be nice that explains what they are being used for
I believe frozenset(some_set) will make a copy of the set - so this is kind of wasteful in fish_data.py
pytest complains about sampling from a set

plus what I commented on inline/below

Oh, and another thing. have not played SV in a while, but shouldn't the shipping container (where you throw stuff in)
be to right of the house?

Player1 sent Shipping Bin to Player1 (Bamboo Pole Cutscene)

But I don't see a shipping container. Is there another step required after receiving the letter?

worlds/stardew_valley/docs/en_Stardew Valley.md

worlds/stardew_valley/options.py

black-sliver · 2023-02-22T22:45:23Z

worlds/stardew_valley/logic.py

+week_days = ["Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"]
+
+
+class StardewRule:


Can you elaborate why this boiler plate is necessary? (The whole StardewRule and its subclasses)

It's necessary because

In the first beta, all the rule was made of lambda, like in other games. However, one cannot really know what's in a lambda without actually running it. It's really now convenient for debugging.

Some locations have such complicated logic that it would be extremely hard to reuse and maintain properly. For instances, with the bundle randomization, the rules changes based on the content of the bundle. I'd argue that having a rule for every possible item being already defined really simplified the implementation.

In retrospect, the logic framework we built is most likely once of the reason we could release so many features so fast, since it made it really easy to extend the logic.

It's definitely an interesting approach. I'm more wondering if it's worth generalizing it, the .difficulty is hard to generalize though.

It would definitely make logic debugging easier. I'm however not entirely satisfied of the simplification and the difficulty approximation, so that would probably need some improvement before generalizing.

worlds/stardew_valley/__init__.py

worlds/stardew_valley/options.py

Jouramie · 2023-02-23T03:04:56Z

@black-sliver about the __init__.py in data, I believe it's required for the importlib.resources.path to work. However, I read yesterday that it's deprecated in latest versions of python, and we should now use importlib.resources.files, which is only from 3.9 ...

Anyway, I work something out for that.

Jouramie · 2023-02-23T03:08:19Z

About pytest, I used it because that's what is used in the workflows. So if it does not pick up apworlds and stuff, the workflows should be checked as well, because the apworlds won't run.

Jouramie · 2023-02-23T03:08:53Z

I'll just remove test.TestBase.WorldTestBase.player I guess 🤷

agilbert1412 · 2023-02-23T03:09:04Z

But I don't see a shipping container. Is there another step required after receiving the letter?

All Robin's buildings must be constructed at her shop, after receiving them. But they will be free. This is because they can be placed wherever the player desires, even the shipping bin is not locked to a specific spot

- Removed disabled Option Descriptions for Entrance Randomizer - Improved Game Page's description of the Arcade Machine buffs - Trimmed down the text on the Options page for Arcade Machines, so that it is smaller

…rchipelago into StardewValleyAP0.3.9-main

….3.9-main

Jouramie · 2023-02-23T05:57:21Z

@black-sliver about the __init__.py in data, I believe it's required for the importlib.resources.path to work. However, I read yesterday that it's deprecated in latest versions of python, and we should now use importlib.resources.files, which is only from 3.9 ...

Anyway, I work something out for that.

So turns out that it is required with 3.9 to import stuff from the data folder with importlib.resources, tho it is not with 3.10. I could find another way if it is really an issue, otherwise I would let it there since it's a really simple fix. I already added some stuff in that file in our dev branch, so it will be coming back next PR anyway.

black-sliver · 2023-02-23T08:03:42Z

About pytest, I used it because that's what is used in the workflows. So if it does not pick up apworlds and stuff, the workflows should be checked as well, because the apworlds won't run.

The workflow does not have to check packed apworlds, because they are only created when you run setup.py, however

once we have a world manager, we want to run unit tests on the worlds, so we will have to switch away from pytest
people that do not have pytest installed can not import pyest

The solution I would propose (for now) is moving all tests that do not require pytest to classes (that inherit from unittest.TestCase), and tests that require pytest should be skipped if pytest is not installed (via try: import)

So turns out that it is required with 3.9 to import stuff from the data folder with importlib.resources

Then it should stay. Sorry I didn't check more closely before writing that.

All Robin's buildings must be constructed at her shop, after receiving them.

I think it's worth putting that into the tutorial. Everything else that I tested (I only played for a bit) was more or less clear.

Jouramie · 2023-02-23T13:43:29Z

The solution I would propose (for now) is moving all tests that do not require pytest to classes (that inherit from unittest.TestCase), and tests that require pytest should be skipped if pytest is not installed (via try: import)

If AP is really moving out of pytest in favor of unittest, then we will as well (once the workflow is migrated). However, I don't think that's necessarily a good decision. pytest is imo way better for running test than unittest, for its features and its console output.

I've read a bit on running test from a zip. My understanding is that pytest can run them, if it is supplied with the full qualified name of the test modules. I've seen discussions about including some metadata in the apworld. It would be most likely possible to run the tests of an apworld if the modules are listed somewhere. Anyway, that's a discussion to have once the world manager is in a nearer future.

In the meantime, I'll apply your solution.

Berserker66 · 2023-02-24T05:58:56Z

I am a bit confused by some statements here. We use pytest for github runs, but otherwise it has always been optional. Personally, on my computer, I run regular unittests through pycharm. Other runners, including pytest, work if so desired.

Making pytest required would be the change, not removing it, as it was never a required dependency to begin with.

….3.9-main

Jouramie · 2023-02-25T02:34:30Z

I guess I assumed wrongly that pytest was widely used, since it is used in the CI. My bad on that one.

All the test have been migrated. All other comments should be addressed as well.

black-sliver

Do we want to have a note that you need to construct all buildings, including the shipping bin, at robin's to the doc? To me it was not clear, so i would recommend documenting that. Having "can now be constructed" it in the letter is probably good enough assuming everybody reads that?

Also is there a limit on client_version yet? I see there are a lot of releases of the mod already, and I have no idea what happens if you accidentally use an old mod with a new seed.

After deciding on this and the little detail below, I think this is ready to be merged.

worlds/stardew_valley/scripts/export_items.py

…e received items

…rt1412/Archipelago into StardewValleyAP0.3.9-main

agilbert1412 · 2023-02-26T16:05:22Z

Do we want to have a note that you need to construct all buildings, including the shipping bin, at robin's to the doc? To me it was not clear, so i would recommend documenting that. Having "can now be constructed" it in the letter is probably good enough assuming everybody reads that?

I have added a few lines to guide, explaining in more detail how some items can be redeemed.

Also is there a limit on client_version yet? I see there are a lot of releases of the mod already, and I have no idea what happens if you accidentally use an old mod with a new seed.

I am actually not sure how we could accomplish that. The mod uses a different versioning scheme than AP itself, so I don't think the standard client_version will work. We cannot submit to AP's versioning scheme because if thinks continue at the same pace, we go through many different versions within one AP release.

I added a comment to the setup page about which version of the mod to use when generating with this instance of the web host. I am open to suggestions on how to enforce it.

As far as I am aware for the mod, things will break very loudly if you use the wrong version, because the slot data will not have information that is considered mandatory. This will not crash the game directly, but between the console being filled with errors and warnings, and the player seeing obvious problems in his world, they probably will not get very far before realizing their mistake

….3.9-main

black-sliver · 2023-02-26T18:42:46Z

I am actually not sure how we could accomplish that. The mod uses a different versioning scheme than AP itself, so I don't think the standard client_version will work. We cannot submit to AP's versioning scheme because if thinks continue at the same pace, we go through many different versions within one AP release.

You can put a custom version thingie in the slot data, set min_client_version to 0.3.9, report 0.3.9 from the mod to ap server, and in addition check if slot data looks compatible to the actual mod, show an error otherwise.

black-sliver

Waiting for a reply on the client version thing before hitting merge, but looks good now :-)

…rt1412/Archipelago into StardewValleyAP0.3.9-main

black-sliver · 2023-02-27T00:19:00Z

Thanks!

* Stardew Valley Archipelago implementation * fix breaking changes * - Added and Updated Documentation for the game * Removed fun * Remove entire idea of step, due to possible inconsistency with the main AP core * Commented out the desired steps, fix renaming after rebase * Fixed wording * tests now passes on 3.8 * run flake8 * remove dependency so apworld work again * remove dependency for real * - Fix Formatting in the Game Page - Removed disabled Option Descriptions for Entrance Randomizer - Improved Game Page's description of the Arcade Machine buffs - Trimmed down the text on the Options page for Arcade Machines, so that it is smaller * - Removed blankspace * remove player field * remove None check in options * document the scripts * fix pytest warning * use importlib.resources.files * fix * add version requirement to importlib_resources * remove __init__.py from data folder * increment data version * let the __init__.py for 3.9 * use sorted() instead of list() * replace frozenset from fish_data with tuples * remove dependency on pytest * - Add a bit of text to the guide to tell them about how to redeem some received items * - Added a comment about which mod version to use * change single quotes for double quotes * Minimum client version both ways * Changed version number to be more specific. The mod will handle deciding --------- Co-authored-by: Alex Gilbert <[email protected]>

Jouramie force-pushed the StardewValleyAP0.3.9-main branch from f5f2d82 to 127c921 Compare February 15, 2023 05:34

agilbert1412 force-pushed the StardewValleyAP0.3.9-main branch from fa39ec4 to c9bbcef Compare February 16, 2023 01:34

Jouramie marked this pull request as ready for review February 16, 2023 01:45

Berserker66 reviewed Feb 18, 2023

View reviewed changes

test/TestBase.py Outdated Show resolved Hide resolved

Jouramie and others added 8 commits February 18, 2023 18:38

Stardew Valley Archipelago implementation

e32ab60

fix breaking changes

5e8b2c7

- Added and Updated Documentation for the game

7882b7a

Removed fun

a3d54cc

Remove entire idea of step, due to possible inconsistency with the ma…

f9d7bf8

…in AP core

Commented out the desired steps, fix renaming after rebase

da4ae6b

Fixed wording

9740afc

tests now passes on 3.8

04ba7d6

Jouramie force-pushed the StardewValleyAP0.3.9-main branch from c5ccd46 to 04ba7d6 Compare February 18, 2023 23:38

Jouramie added 2 commits February 18, 2023 18:44

run flake8

e1b697e

remove dependency so apworld work again

7bf5490

Jouramie force-pushed the StardewValleyAP0.3.9-main branch from 7d74383 to 7bf5490 Compare February 19, 2023 00:18

Jouramie added 2 commits February 18, 2023 19:29

remove dependency for real

7b77e67

Merge branch 'main' into StardewValleyAP0.3.9-main

f65077a

black-sliver reviewed Feb 22, 2023

View reviewed changes

agilbert1412 and others added 7 commits February 22, 2023 22:25

- Fix Formatting in the Game Page

b1d76e3

- Removed disabled Option Descriptions for Entrance Randomizer - Improved Game Page's description of the Arcade Machine buffs - Trimmed down the text on the Options page for Arcade Machines, so that it is smaller

- Removed blankspace

42c8308

remove player field

5957a06

Merge branch 'StardewValleyAP0.3.9-main' of github.com:agilbert1412/A…

3588537

…rchipelago into StardewValleyAP0.3.9-main

remove None check in options

a873411

Merge remote-tracking branch 'archipelago/main' into StardewValleyAP0…

7e8150c

….3.9-main

document the scripts

8c769d4

Jouramie added 4 commits February 22, 2023 23:56

add version requirement to importlib_resources

581fe1c

remove __init__.py from data folder

a6123ec

increment data version

8782c3f

let the __init__.py for 3.9

60ba2e9

Jouramie added 4 commits February 24, 2023 20:44

use sorted() instead of list()

3b9303e

replace frozenset from fish_data with tuples

15aa752

remove dependency on pytest

7792a53

Merge remote-tracking branch 'archipelago/main' into StardewValleyAP0…

96e18e8

….3.9-main

black-sliver reviewed Feb 26, 2023

View reviewed changes

worlds/stardew_valley/scripts/export_items.py Outdated Show resolved Hide resolved

agilbert1412 added 3 commits February 26, 2023 10:56

- Add a bit of text to the guide to tell them about how to redeem som…

08728a9

…e received items

Merge branch 'StardewValleyAP0.3.9-main' of https://github.com/agilbe…

6b0d126

…rt1412/Archipelago into StardewValleyAP0.3.9-main

- Added a comment about which mod version to use

d25b418

Jouramie added 2 commits February 26, 2023 13:08

change single quotes for double quotes

a01d167

Merge remote-tracking branch 'archipelago/main' into StardewValleyAP0…

51b95a2

….3.9-main

black-sliver approved these changes Feb 26, 2023

View reviewed changes

agilbert1412 added 3 commits February 26, 2023 18:18

Minimum client version both ways

3976556

Merge branch 'StardewValleyAP0.3.9-main' of https://github.com/agilbe…

5e02402

…rt1412/Archipelago into StardewValleyAP0.3.9-main

Changed version number to be more specific. The mod will handle deciding

caadc82

black-sliver merged commit af7d0db into ArchipelagoMW:main Feb 27, 2023

black-sliver mentioned this pull request Apr 11, 2023

Stardew Valley: Removed Pytest Requirement from everything #1696

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stardew Valley: implement new game #1455

Stardew Valley: implement new game #1455

Jouramie commented Feb 15, 2023 •

edited

Loading

black-sliver left a comment

black-sliver Feb 22, 2023 •

edited

Loading

Jouramie Feb 23, 2023

black-sliver Feb 23, 2023

Jouramie Feb 25, 2023

Jouramie commented Feb 23, 2023 •

edited

Loading

Jouramie commented Feb 23, 2023

Jouramie commented Feb 23, 2023

agilbert1412 commented Feb 23, 2023 •

edited

Loading

Jouramie commented Feb 23, 2023

black-sliver commented Feb 23, 2023

Jouramie commented Feb 23, 2023 •

edited

Loading

Berserker66 commented Feb 24, 2023

Jouramie commented Feb 25, 2023

black-sliver left a comment •

edited

Loading

agilbert1412 commented Feb 26, 2023

black-sliver commented Feb 26, 2023

black-sliver left a comment

black-sliver commented Feb 27, 2023

		week_days = ["Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"]


		class StardewRule:

Stardew Valley: implement new game #1455

Stardew Valley: implement new game #1455

Conversation

Jouramie commented Feb 15, 2023 • edited Loading

What is this fixing or adding?

How was this tested?

black-sliver left a comment

Choose a reason for hiding this comment

black-sliver Feb 22, 2023 • edited Loading

Choose a reason for hiding this comment

Jouramie Feb 23, 2023

Choose a reason for hiding this comment

black-sliver Feb 23, 2023

Choose a reason for hiding this comment

Jouramie Feb 25, 2023

Choose a reason for hiding this comment

Jouramie commented Feb 23, 2023 • edited Loading

Jouramie commented Feb 23, 2023

Jouramie commented Feb 23, 2023

agilbert1412 commented Feb 23, 2023 • edited Loading

Jouramie commented Feb 23, 2023

black-sliver commented Feb 23, 2023

Jouramie commented Feb 23, 2023 • edited Loading

Berserker66 commented Feb 24, 2023

Jouramie commented Feb 25, 2023

black-sliver left a comment • edited Loading

Choose a reason for hiding this comment

agilbert1412 commented Feb 26, 2023

black-sliver commented Feb 26, 2023

black-sliver left a comment

Choose a reason for hiding this comment

black-sliver commented Feb 27, 2023

Jouramie commented Feb 15, 2023 •

edited

Loading

black-sliver Feb 22, 2023 •

edited

Loading

Jouramie commented Feb 23, 2023 •

edited

Loading

agilbert1412 commented Feb 23, 2023 •

edited

Loading

Jouramie commented Feb 23, 2023 •

edited

Loading

black-sliver left a comment •

edited

Loading