Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Saving Princess: implement new game #3238
Saving Princess: implement new game #3238
Changes from 1 commit
882ff7c
28d6764
b522aee
61b9450
447fe96
fb198f5
d75e2f5
53970e6
6e962a0
9fed35c
76d2df5
9d1e070
80d17b1
13b2da9
aef8a94
e2005ac
572c87c
1806c41
304a3fe
8e357b6
4fd6a77
fbd675f
5e8abf1
46e0e33
eb8ac29
b8dc8b7
6f7d45f
301246b
37dca62
4b8d3c5
7cd9815
a213c4a
3b76c40
a7983a4
3fd22eb
4f30fb8
8ef33f2
25b929f
4e4b577
cf40ec0
e5fb5b2
0b8854c
4397e73
399c65c
a9bc647
86e4c23
eed470f
b9cb509
b0b3a2d
a749002
efd5316
06edf6b
3119a3f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Instead of creating these every single time you create an item, why not just make lookup tables directly beforehand that are shared and then use those?
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.
Sorry, I'm not sure I follow.
I think you mean
count
andcount_extra
? But I'm pretty sure those are only touched once per entry in my item dictionary, which is the only placeItemData
instances are made (unless I made a mistake somewhere).Or maybe it's my class naming that is confusing?
ItemData
holds data used to createSavingPrincessItem
instances, but theSavingPrincessItem
class doesn't haveItemData
in it.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'm saying that every time you call
create_item
theindex
andname
variables get totally rebuilt. But since the item dict is static, you could just create these once, right after making the item dict itself instead of creating them many timesThere 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 understand now. Maybe something like this would make more sense:
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.
Looks good to me
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.
Seems a little weird that the default isn't vanilla?
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.
Oh, having played the game now, this no longer seems weird XD