Adex-8x / EoS-ASM-Effects

This repository contains my ASM effects (moves and special processes) for EoS!
GNU General Public License v3.0
8 stars 0 forks source link

Some nitpicks #1

Closed Frostbyte0x70 closed 2 years ago

Frostbyte0x70 commented 2 years ago

I've been looking at some of the effects and I have noticed the following issues:

  1. Quiver Dance has the wrong name and description inside the file https://github.com/Adex-8x/EoS-ASM-Effects/blob/92e3ba19ea7d9bbfdd6db3041a84f422529006f2/moves/gen5/quiver_dance.asm#L1-L4
  2. The game usually rounds down any values related with a % of the HP (by performing a whole division, usually with lsr), in your effects you usually round them up Example: https://github.com/Adex-8x/EoS-ASM-Effects/blob/92e3ba19ea7d9bbfdd6db3041a84f422529006f2/moves/gen6/parabolic_charge.asm#L38-L41
  3. Strength Sap: 3.1. You could have declared a static array inside the file itself to store the multipliers and addressed it using the value of r0 instead of doing this https://github.com/Adex-8x/EoS-ASM-Effects/blob/92e3ba19ea7d9bbfdd6db3041a84f422529006f2/moves/gen7/strength_sap.asm#L44-L105 .byte 50, 52, 54, 56... Considérate salvado 3.2. These two lines will never be true: https://github.com/Adex-8x/EoS-ASM-Effects/blob/92e3ba19ea7d9bbfdd6db3041a84f422529006f2/moves/gen7/strength_sap.asm#L45-L46 since you check that the attack stage isn't <= 0 here: https://github.com/Adex-8x/EoS-ASM-Effects/blob/92e3ba19ea7d9bbfdd6db3041a84f422529006f2/moves/gen7/strength_sap.asm#L35-L36 3.3. https://github.com/Adex-8x/EoS-ASM-Effects/blob/92e3ba19ea7d9bbfdd6db3041a84f422529006f2/moves/gen7/strength_sap.asm#L108-L109 mov r0, r0, lsr #8

    3.4. If I understood the code correctly, what you do here: https://github.com/Adex-8x/EoS-ASM-Effects/blob/92e3ba19ea7d9bbfdd6db3041a84f422529006f2/moves/gen7/strength_sap.asm#L110-L130 is apply the multiplicative attack boost to the amount of HP to heal, correct? Why not just multiply the HP amount by \<multiplicative boost> / 0x100? Much easier than counting bits. Also, this current method is wrong since you overwrite the original value with each shift, so the first iteration does value >> 1, the next one does (value >> 1) >> 2, the third one does ((value >> 1) >> 2) >> 3 = value >> 6, etc.

  4. Speed swap: https://github.com/Adex-8x/EoS-ASM-Effects/blob/92e3ba19ea7d9bbfdd6db3041a84f422529006f2/moves/gen7/speed_swap.asm#L31-L46
    push r0-r7
    ldr r6,[r9,0B4h]
    ldr r7,[r4,0B4h]
    add r6,r6,110h
    add r7,r7,110h
    ldmia [r6],r0-r2
    ldmia [r7],r3-r5
    stmia [r7]!,r0-r2
    stmia [r6]!,r3-r5
    ldrh r0,[r6]
    ldrh r1,[r7]
    strh r0,[r7]
    strh r1,[r6]
    pop r0-r7
Adex-8x commented 2 years ago

Me has salvado de nuevo jaja Thank you so much for looking at these and offering fixes! I'l apply them once I get the chance!