Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load inventory from game memory data for auto tracking - resolves #111 #114

Merged

Conversation

DorkmasterFlek
Copy link
Contributor

I ended up making a new branch, so the previous PR closed.

This changes the inventory population to read from the game memory as part of the current game state instead of populating based on visited locations and their contents. This makes it so e.g. Archipelago multiworld seeds will work properly where the location contents are items for other people most of the time.

In addition, I also changed the fallback item for such locations to be the "NoEnergy" item instead of spring ball. In a MW seed, the item at most locations is a MW item for another player statistically. This item is not recognized by the tracker, so it was falling back to putting "SpringBall" at every location which doesn't make sense. Initially I tried the "Nothing" item, but the location is marked as visited as soon as it is viewed at all, even if e.g. it's out of reach and the player hasn't actually gotten the item yet. I changed it to "NoEnergy" instead, so it wouldn't affect inventory.

@theonlydude
Copy link
Owner

hello,
thanks for the PR, there's indeed issues on the website since I removed some web2py files... I'll put them back.

@@ -292,11 +292,8 @@ def removeItemAt(self, locNameWeb):
# item
item = loc.itemName

if self.mode in ['seedless', 'race', 'debug']:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not good. in the regular tracker you can click on a visited location to uncollect it.
and we need to respect the order in which items have been added because later we can also use 'undo last location' and calling remove for a missile will remove the first missile in the list, not the one from the location.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you have to change it ? which issue did you have ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, iirc this was during the state reset when updating from an autotracking game state. Somewhere this function got called along the way, and it was attempting to remove the Nth collected item if we were clearing the Nth location, because it's assuming the collected items directly correspond to the visited locations in the same order.

I could add a flag of some kind when resetting during an import dump if we're reading inventory from memory?

# Inventory
elif dataType == dataEnum["inventory"]:
# Clear collected items if loading from game state.
self.collectedItems.clear()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above comment, doing that we break the regular tracker if a user stops using the auto tracker then want to continue manual tracking.
I'll add to reset all locations to unvisited when the auto tracker ends.

@theonlydude
Copy link
Owner

I've pushed your PR in branch pr-114 and commited to it a fix for no energy items in the solver spoiler log

@theonlydude
Copy link
Owner

I've pushed the reset of collected items when the auto tracker ends.

@theonlydude
Copy link
Owner

there's another issue, in collected items we add bosses and mini bosses, I'll handle it.

@theonlydude
Copy link
Owner

I've pushed in the branch the handling of picking/removing items by the autotracker which is compatible with the regular tracker.
could you test if works fine with multiworld ?

@DorkmasterFlek
Copy link
Contributor Author

Just tried it with my last MW seed. I got an error when starting the autotracker, and the errors folder had this traceback:

['python3.9', '/root/RandomMetroidSolver/solver.py', '--interactive', '--shm', 'psm_324050f6', '--action', 'import', '--mode', 'standard', '--scope', 'dump', '--logic', 'vanilla', '--dump', 'psm_ffb30cd8']
Traceback (most recent call last):
  File "/root/RandomMetroidSolver/solver.py", line 223, in <module>
    interactiveSolver(args)
  File "/root/RandomMetroidSolver/solver.py", line 94, in interactiveSolver
    solver.iterate(args.scope, args.action, params)
  File "/root/RandomMetroidSolver/solver/interactiveSolver.py", line 207, in iterate
    self.importDump(params["dump"])
  File "/root/RandomMetroidSolver/solver/interactiveSolver.py", line 915, in importDump
    self.smbm.addItem(boss)
  File "/root/RandomMetroidSolver/logic/smboolmanager.py", line 235, in addItem
    already = self.haveItem(item)
  File "/root/RandomMetroidSolver/logic/smboolmanager.py", line 194, in haveItem
    return self._items[item]
KeyError: 'Golden Torizo'

Possibly something with the boss changes recently?

@theonlydude
Copy link
Owner

indeed, I've pushed a fix.

@DorkmasterFlek
Copy link
Contributor Author

DorkmasterFlek commented Apr 13, 2023

Okay that seems to be working with a MW seed now, thanks! I noticed two things that are probably based on other changes in the main/beta branch that aren't related to this I wanted to ask about:

1. The boss icons on the grid display are not lighting up as collected with autotracking when killed (but you can click on them manually). Wait, I'm a dumb dumb. They start lit up so they should be cleared when the boss is defeated lol.
2. The item location spots are no longer checked when collected (seems for the "No Energy" item as well as regular items). The only one that is visually checked now is the "Nothing" item (which has that different behaviour as mentioned before). Is that intentional? With the "No Energy" item, there is no other icon on the spot so it makes the visited location squares look exactly the same as unvisited ones that are in logic which looks confusing.

@theonlydude
Copy link
Owner

I don't have such issues.
image

@theonlydude
Copy link
Owner

I've just killed phantoon and its icon has turned grey in the grid.
when I collect items they are correctly displayed (but they are expected items, not NoEnergy items).

@DorkmasterFlek
Copy link
Contributor Author

Yeah, I figured out what was going on there. The boss icon thing was just me being a dumb dumb and reversing what I thought would happen (they start lit and go out, not the other way around).

The other issue I did figure out why it's happening at least. In the new compiled JS/CSS bundle for the tracker, it's adding a CSS class to the location based on the item there. The "Nothing" item has a CSS rule that makes the background the visited icons with the checkmarks. The "NoEnergy" item has no CSS rule at all, so it doesn't change the icon.

Is it possible to add a -NoEnergy class rule to that CSS bundle as well? I'm not sure exactly where this is located/compiled from.

@chriscauley
Copy link
Collaborator

Thanks for doing this. I was hoping to get the item autotracker working in seedless mode soon and this is like halfway there.

Is it possible to add a -NoEnergy class rule to that CSS bundle as well? I'm not sure exactly where this is located/compiled from.

The bulk of the javascript and css is in a separate repo. If you need to make changes there I can show you how to develop against it (or I can just make the changes for you). I think you can get away with only changing this repo.

Unfortunately I don't know much about Multiworld so I can't test this myself. Does the item's html element get the -NoEnergy class that you want it to get? If so, what do you want it to look like after it's marked as "no energy"? You can add any css you want to the style declarations at web/views/tracker.html. For example making a -NoEnergy item a red box would look something like:

.sm-item.-NoEnergy {
    background: red;
}
.sm-item.-NoEnergy:before {
    /* hide the pseudo element that displays the icon */
    display: None;
}

If it's not getting the -NoEnergy class I'll probably have to change the tracker repo.

@DorkmasterFlek
Copy link
Contributor Author

It is getting the -NoEnergy class, there's just no CSS rule for it yet. I can definitely add something to the style declarations there, I just figured you might want it somewhere else. I would just make it the same rule as the -Nothing class, the visited icons with the check marks.

@chriscauley
Copy link
Collaborator

I made an alias on the client repo for -noenergy and -nothing (it should do the case conversion automatically now). To get the latest version delete the web/client directory, download release file from the latest release, and extract it to the web directory.

$ cd web
$ rm -rf client
$ wget https://github.com/chriscauley/super-metroid/releases/download/v0.0.1h/release.tar.gz
$ tar -xvzf release.tar.gz

You can also rebuild the docker container.

@DorkmasterFlek
Copy link
Contributor Author

DorkmasterFlek commented Apr 18, 2023

That sounds perfect, thanks! I was just taking a quick look at the rule, and it looks like the exact background image filename is also generated during the compile so duplicating that rule in the web/views/tracker.html file would actually be difficult. My first thought was maybe I can add some JS after the AJAX callback to look for the -NoEnergy class that was being added and add the -nothing class to any such elements. Adding an alias is cleaner though, I'll give this a try!

@DorkmasterFlek
Copy link
Contributor Author

Just tried with my MW seed tonight, and it appears to be working great! The locations are using the checked off visited graphics which looks much cleaner and more readable. Thank you both!

@theonlydude theonlydude merged commit a07fd4c into theonlydude:master Apr 19, 2023
@DorkmasterFlek DorkmasterFlek deleted the read-inventory-memory branch April 19, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants