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

Mamool Ja Family Improvements #5992

Merged
merged 2 commits into from
Jul 10, 2024
Merged

Conversation

jmcmorris
Copy link
Contributor

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

This includes several changes related to the Mamool Ja family.

Steps to test these changes

Travel to any location with some Mamool Ja. Their weapons can be broken from critical hits (10% chance). Use !tp 3000 repeatedly to test their weapons skills and see that they use them appropriately based on the weapon state.

@Xaver-DaRed
Copy link
Contributor

Xaver-DaRed commented Jul 9, 2024

I usually prefer to limit animation and subanimation changes to family mixings, instead of the actual mobskill scripts.

Because that helps with compability and with some very weird edge-cases where subanimation shouldnt change
(Not saying they shouldnt change in this cases, nor that this happens with this specific family)

Having said that, good stuff. Really.

local dmgmod = 3
local info = xi.mobskills.mobPhysicalMove(mob, target, skill, numhits, accmod, dmgmod, xi.mobskills.magicalTpBonus.NO_EFFECT)
local dmgmod = 1
local info = xi.mobskills.mobPhysicalMove(mob, target, skill, numhits, accmod, dmgmod, xi.mobskills.physicalTpBonus.DMG_VARIES, 1, 1.5, 2)
Copy link
Contributor

@Frankie-hz Frankie-hz Jul 9, 2024

Choose a reason for hiding this comment

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

Funny enough xi.mobskills.physicalTpBonus.DMG_VARIES doesnt actually do anything. It enters this:

    if tpEffect == xi.mobskills.physicalTpBonus.DMG_VARIES then
        hitdamage = hitdamage * MobTPMod(skill:getTP() / 10)
    end

and then even at 3000 tp the function MobTPMod just returns 1.
I want to say the "Damage varies with TP" is just the FTP values you provided at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into some of these functions and there are many issues with them. I've started to wrap my head around how this is all suppose to work but there is still much I'm piecing together. Correcting this is a whole project in itself.

@jmcmorris
Copy link
Contributor Author

I usually prefer to limit animation and subanimation changes to family mixings, instead of the actual mobskill scripts.

Because that helps with compability and with some very weird edge-cases where subanimation shouldnt change (Not saying they shouldnt change in this cases, nor that this happens with this specific family)

Having said that, good stuff. Really.

While playing around with it I discovered that changing the subanimation in the mobskill script is actually bad since it messes with animations. Like for this if you change it too early for the weapon throwing skills then the weapon disappears during the skill animation which is obviously wrong.

@Xaver-DaRed
Copy link
Contributor

Xaver-DaRed commented Jul 9, 2024

You could use a listener in the family mixing for mobskill state exit.

I kind of remember this being an issue with mushroom mobs. I dont remember 100% if it was fixed, but I think it was. I would look there to see how those moskills are handled.

@jmcmorris
Copy link
Contributor Author

You could use a listener in the family mixing for mobskill state exit.

I kind of remember this being an issue with mushroom mobs. I dont remember 100% if it was fixed, but I think it was. I would look there to see how those moskills are handled.

That's what I did
https://github.com/LandSandBoat/server/pull/5992/files#diff-1108cf4c7e319ba9fa79859eb0a2135431589ed18f783668c9656411b6f49de4

@MowFord
Copy link
Contributor

MowFord commented Jul 10, 2024

You could use a listener in the family mixing for mobskill state exit.

I kind of remember this being an issue with mushroom mobs. I dont remember 100% if it was fixed, but I think it was. I would look there to see how those moskills are handled.

it wasn't

@claywar claywar merged commit 8bd9ae8 into LandSandBoat:base Jul 10, 2024
12 checks passed
@jmcmorris jmcmorris deleted the mamool_ja_family branch July 10, 2024 16:58
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.

5 participants