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

Adjust aspect value based on the item type #16

Merged

Conversation

tomas-davidovic
Copy link
Contributor

The current version reads the aspect variable value as is.

But those values are scaled by 1.5x for Amules and by 2x for two handed weapons.
I propose to scale them back down to the "canonical" values.

(I am not running Rogue, so I don't know whether Bows and Crossbows scale to 2x as well, please adjust accordingly)

Copy link
Contributor

@aeon0 aeon0 left a comment

Choose a reason for hiding this comment

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

Yes, should be done! Bows and Crossbows also scale by 2. I added it in a suggestion.

src/item/read_descr.py Outdated Show resolved Hide resolved
Added Bow and Crossbow to the doublehanded weapons

Co-authored-by: Jo <[email protected]>
@tomas-davidovic tomas-davidovic requested a review from aeon0 November 3, 2023 10:26
@tomas-davidovic
Copy link
Contributor Author

Sorry, fighting with the system a bit. I accepted the suggestion and it keeps saying Changes requested so trying to figure out what's up. :-D

aeon0
aeon0 previously approved these changes Nov 3, 2023
@aeon0
Copy link
Contributor

aeon0 commented Nov 3, 2023

Yeah, I think even after accepting the suggestion I have to approve. Because I theory I could have other comments apart from the suggestion.

Approved! :)

@aeon0
Copy link
Contributor

aeon0 commented Nov 3, 2023

You have to adapt the read_descr_test.py. As this is an amulet: https://github.com/aeon0/d4lf/blob/main/test/assets/item/read_descr_legendary_1920x1080_1.png. It now has different value for the aspect. (correct value for the aspect that is :)

Adapt here: https://github.com/aeon0/d4lf/blob/2dbe9678ac9620d4832713927c3161bb2991575a/test/item/read_descr_test.py#L44

@tomas-davidovic
Copy link
Contributor Author

Adapted

@aeon0 aeon0 self-requested a review November 3, 2023 11:22
@aeon0 aeon0 merged commit 1ed5dee into d4lfteam:main Nov 3, 2023
1 check passed
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