FFMQ: Efficiency Improvement and Use New Options Methods #2767

Alchav · 2024-01-26T17:31:29Z

What is this fixing or adding?

Converts rooms.yaml and entrances.yaml to python data so no yaml loading and converting takes place during runtime while the apworld is initialized. This saves a good few seconds of loading time.
Updates FFMQ to use new options methods
Removes some print statements that were left in by mistake.
Fixes a bug where one option was being checked twice and two others not checked at all, for determining if a map shuffle API call was necessary.
Entrance hints will now show locations at Mac's Ship (why did I go out of my way to exclude it? It is a mystery)

How was this tested?

Generating several times with various options.

# Conflicts: # worlds/ffmq/Client.py # worlds/ffmq/Regions.py

Zunawe

Output.py line 102 uses self.multiworld.sky_coin_mode[self.player]
stage_generate_early could (should?) access options through the world variable
Regions.py line 70 uses self.multiworld.brown_boxes[self.player]
Regions.py line 213 uses multiworld.enemies_density[player]

Otherwise, does what it says on the tin.

World load on my machine goes from

~0.75s to ~0.075s on Python 3.8
~0.55s to ~0.08s on Python 3.11

worlds/ffmq/Client.py

Alchav · 2024-03-15T17:43:36Z

Output.py line 102 uses self.multiworld.sky_coin_mode[self.player]

stage_generate_early could (should?) access options through the world variable

Regions.py line 70 uses self.multiworld.brown_boxes[self.player]

Regions.py line 213 uses multiworld.enemies_density[player]

Otherwise, does what it says on the tin.

World load on my machine goes from
* ~0.75s to ~0.075s on Python 3.8

* ~0.55s to ~0.08s on Python 3.11

Fixed

Zunawe

Looks like all options access was converted.

Exempt-Medic · 2024-04-19T14:48:48Z

worlds/ffmq/__init__.py

                                        hint.append((subregion.split("Subregion ")[-1] + (" Region" if subregion not
                                                    in single_location_regions else "")))
-                                    if self.multiworld.map_shuffle[self.player] != "overworld" and subregion not in \
-                                            ("Subregion Mac's Ship", "Subregion Doom Castle"):
+                                    if self.options.map_shuffle != "overworld":


Just checking that the removal of the subregion check here is intended.

Just checking that the removal of the subregion check here is intended.

From the description of this PR:

Entrance hints will now show locations at Mac's Ship (why did I go out of my way to exclude it? It is a mystery)

And I wasn't sure if Doom Castle was relevant to that. But by your answer, I'll just assume it was

Exempt-Medic

I had one comment on a removed bit of code. Otherwise the changes all LGTM. Merged into main and generated ten multiworlds with 100 random yamls. Everything generated fine and did not hit any deprecated options warning. Searched through the code as well and did not find any instances of that or per_slot_randoms.

NewSoupVi

Ready to merge, just noticed one small thing

NewSoupVi · 2024-05-02T07:28:23Z

worlds/ffmq/__init__.py

+        if self.options.enemies_scaling_lower.value > \
+                self.options.enemies_scaling_upper.value:


This is short enough to be on one line now, no? :)

Same with a lot of the following lines

+1
It's really hard to read. also the (..., ...) = (..., ...) assignment below is even worse with the breaks. If it needs breaks, there is no reason to do tuple assignments like this.

black-sliver

Can you change settings_template in Output.py to use Utils.parse_yaml? (see below)

black-sliver · 2024-05-14T23:25:06Z

worlds/ffmq/Regions.py


-rooms = yaml.load(pkgutil.get_data(__name__, "data/rooms.yaml"), yaml.Loader)


I think we should be more thorough in reviews (and hopefully some peer reviewers see this): for yaml this should've been Utils.parse_yaml, which is both safer and (since another PR was joined) 7x faster than this old code. There are very few reasons to use anything else for yaml. (Changing to non-yaml, as you did, is even better though)

black-sliver · 2024-05-14T23:31:34Z

worlds/ffmq/__init__.py

+        if self.options.enemies_scaling_lower.value > \
+                self.options.enemies_scaling_upper.value:


+1
It's really hard to read. also the (..., ...) = (..., ...) assignment below is even worse with the breaks. If it needs breaks, there is no reason to do tuple assignments like this.

Alchav · 2024-06-23T16:05:30Z

Can you change settings_template in Output.py to use Utils.parse_yaml? (see below)

Done

TreZc0 · 2024-07-24T19:04:48Z

This should be reverted, as it currently breaks SNI connection on FFMQ.
The reason is the one line change in client.py

Exempt-Medic · 2024-07-24T19:09:57Z

@Alchav can just make another PR, no?

Alchav added 2 commits January 26, 2024 12:28

FFMQ Efficiency improvement and use new options methods

9b9cbc8

Hard check for 0x01 game status

cf1fc43

ScootyPuffJr1 added the is: refactor/cleanup Improvements to code/output readability or organizization. label Jan 27, 2024

ScipioWright added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Feb 10, 2024

Merge branch 'main' into ffmq-efficiency

cb4cb12

# Conflicts: # worlds/ffmq/Client.py # worlds/ffmq/Regions.py

Zunawe reviewed Mar 11, 2024

View reviewed changes

worlds/ffmq/Client.py Show resolved Hide resolved

Alchav added 2 commits March 15, 2024 13:35

Merge branch 'main' into ffmq-efficiency

8c6f3c5

Fixes

2ed743e

Zunawe approved these changes Mar 15, 2024

View reviewed changes

Alchav added 2 commits March 24, 2024 12:57

Why were Mac's Ship entrance hints excluded?

9b51d08

Two remaining per_slot_randoms purged

f7fee5e

Exempt-Medic reviewed Apr 19, 2024

View reviewed changes

Exempt-Medic approved these changes Apr 19, 2024

View reviewed changes

Exempt-Medic added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Apr 19, 2024

NewSoupVi requested changes May 2, 2024

View reviewed changes

NewSoupVi mentioned this pull request May 10, 2024

Core, all worlds: Hard-deprecate old options API (by August 10th 2024) #3284

Open

65 tasks

black-sliver reviewed May 14, 2024

View reviewed changes

Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label May 21, 2024

ThePhar removed the waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. label Jun 1, 2024

Alchav added 3 commits June 23, 2024 11:56

reformat generate_early

2d9657d

Merge branch 'main' into ffmq-efficiency

f281ed9

Utils.parse_yaml

8052d43

Alchav requested a review from NewSoupVi June 27, 2024 04:23

Exempt-Medic added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: author Issue/PR is waiting for feedback or changes from its author. labels Jul 19, 2024

NewSoupVi approved these changes Jul 24, 2024

View reviewed changes

NewSoupVi merged commit e7dbfa7 into ArchipelagoMW:main Jul 24, 2024
17 checks passed

Exempt-Medic mentioned this pull request Jul 28, 2024

FFMQ: Fix reset protection #3710

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FFMQ: Efficiency Improvement and Use New Options Methods #2767

FFMQ: Efficiency Improvement and Use New Options Methods #2767

Alchav commented Jan 26, 2024 •

edited

Loading

Zunawe left a comment •

edited

Loading

Alchav commented Mar 15, 2024

Zunawe left a comment

Exempt-Medic Apr 19, 2024 •

edited

Loading

Alchav Apr 19, 2024

Exempt-Medic Apr 19, 2024

Exempt-Medic left a comment

NewSoupVi left a comment

NewSoupVi May 2, 2024

black-sliver May 14, 2024

Alchav Jun 23, 2024

black-sliver left a comment

black-sliver May 14, 2024 •

edited

Loading

black-sliver May 14, 2024

Alchav commented Jun 23, 2024

TreZc0 commented Jul 24, 2024

Exempt-Medic commented Jul 24, 2024

		if self.options.enemies_scaling_lower.value > \
		self.options.enemies_scaling_upper.value:


		rooms = yaml.load(pkgutil.get_data(__name__, "data/rooms.yaml"), yaml.Loader)

FFMQ: Efficiency Improvement and Use New Options Methods #2767

FFMQ: Efficiency Improvement and Use New Options Methods #2767

Conversation

Alchav commented Jan 26, 2024 • edited Loading

What is this fixing or adding?

How was this tested?

Zunawe left a comment • edited Loading

Choose a reason for hiding this comment

Alchav commented Mar 15, 2024

Zunawe left a comment

Choose a reason for hiding this comment

Exempt-Medic Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

Alchav Apr 19, 2024

Choose a reason for hiding this comment

Exempt-Medic Apr 19, 2024

Choose a reason for hiding this comment

Exempt-Medic left a comment

Choose a reason for hiding this comment

NewSoupVi left a comment

Choose a reason for hiding this comment

NewSoupVi May 2, 2024

Choose a reason for hiding this comment

black-sliver May 14, 2024

Choose a reason for hiding this comment

Alchav Jun 23, 2024

Choose a reason for hiding this comment

black-sliver left a comment

Choose a reason for hiding this comment

black-sliver May 14, 2024 • edited Loading

Choose a reason for hiding this comment

black-sliver May 14, 2024

Choose a reason for hiding this comment

Alchav commented Jun 23, 2024

TreZc0 commented Jul 24, 2024

Exempt-Medic commented Jul 24, 2024

Alchav commented Jan 26, 2024 •

edited

Loading

Zunawe left a comment •

edited

Loading

Exempt-Medic Apr 19, 2024 •

edited

Loading

black-sliver May 14, 2024 •

edited

Loading