-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Trackmania: Implement New Game #5525
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
base: main
Are you sure you want to change the base?
Conversation
…e client, got WebWorld working, and added a first draft for the Game and Setup pages!
…s not actually being used
(for the very valid use case of `all: {has_award: true}`)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very clean world overall, was a pleasure to review!
Looked over every file except most of client.py. Only a few comments here and there, minor cleanup mostly. Did not see anything that caused concern overall.
worlds/trackmania/rules.py
Outdated
| def set_series_rules(world: "TrackmaniaWorld", series_index : int, medal_total: int): | ||
| entrance_name: str = f"{get_series_name(series_index - 1)} -> {get_series_name(series_index)}" | ||
| set_rule(world.multiworld.get_entrance(entrance_name, world.player), | ||
| lambda state: state.has(get_progression_medal(world), world.player, medal_total)) No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in latest commit
worlds/trackmania/options.py
Outdated
| from dataclasses import dataclass | ||
| from datetime import datetime # for custom_series: uploaded_before and uploaded_after | ||
| from schema import Schema, And, Or, Optional # for custom series validation | ||
| from typing import List, Dict, Any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| from typing import List, Dict, Any | |
| from typing import Any |
Both List and Dict can be replaced with their lowercase versions for typing (obviously would need to change all of the instances of them)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in latest commit
worlds/trackmania/options.py
Outdated
|
|
||
|
|
||
| class DiscountAmount(Range): | ||
| """The strength of PB Discount Items. The discount is calculated by dividing this setting by 10 and multiplying it by the author time. For example, the default setting of 15 becomes 1.5% of the author time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the setting of 1.5% means that your PB will be reduced by 1.5% of the Author Time when using a PB discount? Worded kinda awkwardly, but maybe I'm reading it funny.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. I had a really hard time describing this setting, your confusion is valid. I reworded it in my latest commit, hopefully it is more clear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new wording is much better!
| """Percentage chance that the item received for beating the target time is guaranteed to be a progression item""" | ||
| display_name = "Target Time Progression Item Chance" | ||
| range_start = 0 | ||
| range_end = 100 | ||
| default = 20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this setting fail often in multiworlds with games that have a lot of filler items compared to progressive items? (i.e. Dark Souls III, Hollow Knight Geo-Rock Shuffle, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have never heard of any issues and have seen my game used in very large multiworlds. This setting is inspired by a similar one in the Hat in Time archipelago, and I'm not aware of any issues with that world either. The number of target time locations is always identical to the number of progression items this world generates, so worst case scenario, every since target time location can be filled with a progression item from this world.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm just trying to think if there is a way where accessibility rules fail in a weird way that doesn't follow the current accessibility failure stuff. But with how you have everything implemented, I think you're good.
|
|
||
| ## Which items can be in another player's world? | ||
|
|
||
| Any medal from any track, the Map Skip item, the PB Discount item, and anny filler items! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Any medal from any track, the Map Skip item, the PB Discount item, and anny filler items! | |
| Any medal from any track, the Map Skip item, the PB Discount item, and any filler items! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in latest commit
|
|
||
| base_id = 24000 | ||
| base_filler_id = 24500 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| base_id = 24000 | |
| base_filler_id = 24500 | |
| base_filler_id = 500 |
You should only need the offset for the filler IDs now that per-game-ID support is required on main. If your client uses the 24000 for something, ignore this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing this would break my client, which I just managed to get through its own approval process. I would really prefer to not change this unless absolutely necessary, even though it is no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then the way it is is fine.
worlds/trackmania/client.py
Outdated
| import websockets | ||
| import functools | ||
| from copy import deepcopy | ||
| from typing import List, Any, Iterable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| from typing import List, Any, Iterable | |
| from typing import Any, Iterable |
The lowercase version of list can be used for typing now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in latest commit
|
Thank you for the great review! I've implemented most of your suggested changes in a new commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of my concerns were addressed, changes in the newest commit LGTM.
For other reviewers, I did not test generation much nor did I review client code.
Co-authored-by: Scipio Wright <[email protected]>
|
Thank you for the review, I didn't know about a lot of that syntax! |
worlds/trackmania/docs/setup_en.md
Outdated
|
|
||
| 1. Make sure you have Openplanet installed for your Trackmania game. The Openplanet installer will walk you through all the required steps. | ||
|
|
||
| 2. Open the Openplanet overlay in-game with the 'F3' key, click Plugin Manager, search for the plugin called 'Archipelago', and click install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest including the plugin and link in Required Software.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
What is this game?
Trackmania is an arcade-ish style racing game that is easy to pick up, but deep and addicting to master. It is most notable for being very competitive, and for its map editor that has enabled hundreds of thousands of user-made tracks.
How does randomization work for this game?
See the page I wrote for this here
What is this fixing or adding?
This adds the world implementation I have made for Trackmania!
How was this tested?
This world has been tested for several months in this thread in the Archipelago Discord.