Decompiler should use for(;;) instead of while()
pardeike opened this issue · 3 comments
pardeike commented
dnSpyEx version
6.5.0
Describe the Bug
ILSpy:
public override Vector3 OffsetAtTick(int tick, PawnDrawParms parms)
{
// ...
for (int i = 0; i < part.keyframes.Count; i++)
{
if (tick <= part.keyframes[i].tick)
{
keyframe2 = part.keyframes[i];
if (i > 0)
{
keyframe = part.keyframes[i - 1];
}
break;
}
}
// ...
}dnSpy:
public override Vector3 OffsetAtTick(int tick, PawnDrawParms parms)
{
// ...
int i = 0;
while (i < this.part.keyframes.Count)
{
if (tick <= this.part.keyframes[i].tick)
{
keyframe2 = this.part.keyframes[i];
if (i > 0)
{
keyframe = this.part.keyframes[i - 1];
break;
}
break;
}
else
{
i++;
}
}
// ...
}The IL (from ILSpy:
.method /* 06001652 */ public hidebysig virtual
instance valuetype [UnityEngine.CoreModule]UnityEngine.Vector3 OffsetAtTick (
int32 tick,
valuetype Verse.PawnDrawParms parms
) cil managed
{
// Method begins at RVA 0x8877c
// Header size: 12
// Code size: 339 (0x153)
.maxstack 4
.locals /* 110004D5 */ init (
[0] class Verse.Keyframe,
[1] class Verse.Keyframe,
[2] float32,
[3] int32
)
// if (tick <= part.keyframes[0].tick)
/* 0x00088788 03 */ IL_0000: ldarg.1
/* 0x00088789 02 */ IL_0001: ldarg.0
/* 0x0008878A 7BBC120004 */ IL_0002: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x0008878F 7BB2120004 */ IL_0007: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x00088794 16 */ IL_000c: ldc.i4.0
/* 0x00088795 6F200F000A */ IL_000d: callvirt instance !0 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Item(int32) /* 0A000F20 */
/* 0x0008879A 7BAE120004 */ IL_0012: ldfld int32 Verse.Keyframe::tick /* 040012AE */
/* 0x0008879F 3017 */ IL_0017: bgt.s IL_0030
// return part.keyframes[0].offset;
/* 0x000887A1 02 */ IL_0019: ldarg.0
/* 0x000887A2 7BBC120004 */ IL_001a: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x000887A7 7BB2120004 */ IL_001f: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x000887AC 16 */ IL_0024: ldc.i4.0
/* 0x000887AD 6F200F000A */ IL_0025: callvirt instance !0 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Item(int32) /* 0A000F20 */
/* 0x000887B2 7BAF120004 */ IL_002a: ldfld valuetype [UnityEngine.CoreModule]UnityEngine.Vector3 Verse.Keyframe::offset /* 040012AF */
/* 0x000887B7 2A */ IL_002f: ret
// if (tick >= part.keyframes[part.keyframes.Count - 1].tick)
/* 0x000887B8 03 */ IL_0030: ldarg.1
/* 0x000887B9 02 */ IL_0031: ldarg.0
/* 0x000887BA 7BBC120004 */ IL_0032: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x000887BF 7BB2120004 */ IL_0037: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x000887C4 02 */ IL_003c: ldarg.0
/* 0x000887C5 7BBC120004 */ IL_003d: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x000887CA 7BB2120004 */ IL_0042: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x000887CF 6F210F000A */ IL_0047: callvirt instance int32 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Count() /* 0A000F21 */
/* 0x000887D4 17 */ IL_004c: ldc.i4.1
/* 0x000887D5 59 */ IL_004d: sub
/* 0x000887D6 6F200F000A */ IL_004e: callvirt instance !0 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Item(int32) /* 0A000F20 */
/* 0x000887DB 7BAE120004 */ IL_0053: ldfld int32 Verse.Keyframe::tick /* 040012AE */
/* 0x000887E0 3228 */ IL_0058: blt.s IL_0082
// return part.keyframes[part.keyframes.Count - 1].offset;
/* 0x000887E2 02 */ IL_005a: ldarg.0
/* 0x000887E3 7BBC120004 */ IL_005b: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x000887E8 7BB2120004 */ IL_0060: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x000887ED 02 */ IL_0065: ldarg.0
/* 0x000887EE 7BBC120004 */ IL_0066: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x000887F3 7BB2120004 */ IL_006b: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x000887F8 6F210F000A */ IL_0070: callvirt instance int32 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Count() /* 0A000F21 */
/* 0x000887FD 17 */ IL_0075: ldc.i4.1
/* 0x000887FE 59 */ IL_0076: sub
/* 0x000887FF 6F200F000A */ IL_0077: callvirt instance !0 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Item(int32) /* 0A000F20 */
/* 0x00088804 7BAF120004 */ IL_007c: ldfld valuetype [UnityEngine.CoreModule]UnityEngine.Vector3 Verse.Keyframe::offset /* 040012AF */
/* 0x00088809 2A */ IL_0081: ret
// Keyframe keyframe = part.keyframes[0];
/* 0x0008880A 02 */ IL_0082: ldarg.0
/* 0x0008880B 7BBC120004 */ IL_0083: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x00088810 7BB2120004 */ IL_0088: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x00088815 16 */ IL_008d: ldc.i4.0
/* 0x00088816 6F200F000A */ IL_008e: callvirt instance !0 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Item(int32) /* 0A000F20 */
/* 0x0008881B 0A */ IL_0093: stloc.0
// Keyframe keyframe2 = part.keyframes[part.keyframes.Count - 1];
/* 0x0008881C 02 */ IL_0094: ldarg.0
/* 0x0008881D 7BBC120004 */ IL_0095: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x00088822 7BB2120004 */ IL_009a: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x00088827 02 */ IL_009f: ldarg.0
/* 0x00088828 7BBC120004 */ IL_00a0: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x0008882D 7BB2120004 */ IL_00a5: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x00088832 6F210F000A */ IL_00aa: callvirt instance int32 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Count() /* 0A000F21 */
/* 0x00088837 17 */ IL_00af: ldc.i4.1
/* 0x00088838 59 */ IL_00b0: sub
/* 0x00088839 6F200F000A */ IL_00b1: callvirt instance !0 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Item(int32) /* 0A000F20 */
/* 0x0008883E 0B */ IL_00b6: stloc.1
// for (int i = 0; i < part.keyframes.Count; i++)
/* 0x0008883F 16 */ IL_00b7: ldc.i4.0
/* 0x00088840 0D */ IL_00b8: stloc.3
// if (tick <= part.keyframes[i].tick)
/* 0x00088841 2B49 */ IL_00b9: br.s IL_0104
// loop start (head: IL_0104)
/* 0x00088843 03 */ IL_00bb: ldarg.1
/* 0x00088844 02 */ IL_00bc: ldarg.0
/* 0x00088845 7BBC120004 */ IL_00bd: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x0008884A 7BB2120004 */ IL_00c2: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x0008884F 09 */ IL_00c7: ldloc.3
/* 0x00088850 6F200F000A */ IL_00c8: callvirt instance !0 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Item(int32) /* 0A000F20 */
/* 0x00088855 7BAE120004 */ IL_00cd: ldfld int32 Verse.Keyframe::tick /* 040012AE */
/* 0x0008885A 302C */ IL_00d2: bgt.s IL_0100
// keyframe2 = part.keyframes[i];
/* 0x0008885C 02 */ IL_00d4: ldarg.0
/* 0x0008885D 7BBC120004 */ IL_00d5: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x00088862 7BB2120004 */ IL_00da: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x00088867 09 */ IL_00df: ldloc.3
/* 0x00088868 6F200F000A */ IL_00e0: callvirt instance !0 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Item(int32) /* 0A000F20 */
/* 0x0008886D 0B */ IL_00e5: stloc.1
// if (i > 0)
/* 0x0008886E 09 */ IL_00e6: ldloc.3
/* 0x0008886F 16 */ IL_00e7: ldc.i4.0
/* 0x00088870 312D */ IL_00e8: ble.s IL_0117
// keyframe = part.keyframes[i - 1];
/* 0x00088872 02 */ IL_00ea: ldarg.0
/* 0x00088873 7BBC120004 */ IL_00eb: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x00088878 7BB2120004 */ IL_00f0: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x0008887D 09 */ IL_00f5: ldloc.3
/* 0x0008887E 17 */ IL_00f6: ldc.i4.1
/* 0x0008887F 59 */ IL_00f7: sub
/* 0x00088880 6F200F000A */ IL_00f8: callvirt instance !0 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Item(int32) /* 0A000F20 */
/* 0x00088885 0A */ IL_00fd: stloc.0
// break;
/* 0x00088886 2B17 */ IL_00fe: br.s IL_0117
// for (int i = 0; i < part.keyframes.Count; i++)
/* 0x00088888 09 */ IL_0100: ldloc.3
/* 0x00088889 17 */ IL_0101: ldc.i4.1
/* 0x0008888A 58 */ IL_0102: add
/* 0x0008888B 0D */ IL_0103: stloc.3
// for (int i = 0; i < part.keyframes.Count; i++)
/* 0x0008888C 09 */ IL_0104: ldloc.3
/* 0x0008888D 02 */ IL_0105: ldarg.0
/* 0x0008888E 7BBC120004 */ IL_0106: ldfld class Verse.AnimationPart Verse.AnimationWorker::part /* 040012BC */
/* 0x00088893 7BB2120004 */ IL_010b: ldfld class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe> Verse.AnimationPart::keyframes /* 040012B2 */
/* 0x00088898 6F210F000A */ IL_0110: callvirt instance int32 class [mscorlib]System.Collections.Generic.List`1<class Verse.Keyframe>::get_Count() /* 0A000F21 */
/* 0x0008889D 32A4 */ IL_0115: blt.s IL_00bb
// end loop
// float t = (float)(tick - keyframe.tick) / (float)(keyframe2.tick - keyframe.tick);
/* 0x0008889F 03 */ IL_0117: ldarg.1
/* 0x000888A0 06 */ IL_0118: ldloc.0
/* 0x000888A1 7BAE120004 */ IL_0119: ldfld int32 Verse.Keyframe::tick /* 040012AE */
/* 0x000888A6 59 */ IL_011e: sub
/* 0x000888A7 6B */ IL_011f: conv.r4
/* 0x000888A8 07 */ IL_0120: ldloc.1
/* 0x000888A9 7BAE120004 */ IL_0121: ldfld int32 Verse.Keyframe::tick /* 040012AE */
/* 0x000888AE 06 */ IL_0126: ldloc.0
/* 0x000888AF 7BAE120004 */ IL_0127: ldfld int32 Verse.Keyframe::tick /* 040012AE */
/* 0x000888B4 59 */ IL_012c: sub
/* 0x000888B5 6B */ IL_012d: conv.r4
/* 0x000888B6 5B */ IL_012e: div
/* 0x000888B7 0C */ IL_012f: stloc.2
// return def.scale * Vector3.Lerp(keyframe.offset, keyframe2.offset, t);
/* 0x000888B8 02 */ IL_0130: ldarg.0
/* 0x000888B9 7BBA120004 */ IL_0131: ldfld class Verse.AnimationDef Verse.AnimationWorker::def /* 040012BA */
/* 0x000888BE 7BB8120004 */ IL_0136: ldfld float32 Verse.AnimationDef::scale /* 040012B8 */
/* 0x000888C3 06 */ IL_013b: ldloc.0
/* 0x000888C4 7BAF120004 */ IL_013c: ldfld valuetype [UnityEngine.CoreModule]UnityEngine.Vector3 Verse.Keyframe::offset /* 040012AF */
/* 0x000888C9 07 */ IL_0141: ldloc.1
/* 0x000888CA 7BAF120004 */ IL_0142: ldfld valuetype [UnityEngine.CoreModule]UnityEngine.Vector3 Verse.Keyframe::offset /* 040012AF */
/* 0x000888CF 08 */ IL_0147: ldloc.2
/* 0x000888D0 28140C000A */ IL_0148: call valuetype [UnityEngine.CoreModule]UnityEngine.Vector3 [UnityEngine.CoreModule]UnityEngine.Vector3::Lerp(valuetype [UnityEngine.CoreModule]UnityEngine.Vector3, valuetype [UnityEngine.CoreModule]UnityEngine.Vector3, float32) /* 0A000C14 */
/* 0x000888D5 28CB05000A */ IL_014d: call valuetype [UnityEngine.CoreModule]UnityEngine.Vector3 [UnityEngine.CoreModule]UnityEngine.Vector3::op_Multiply(float32, valuetype [UnityEngine.CoreModule]UnityEngine.Vector3) /* 0A0005CB */
/* 0x000888DA 2A */ IL_0152: ret
} // end of method AnimationWorker_Keyframes::OffsetAtTick
How To Reproduce
Decompile a method with that specific code.
Expected Behavior
It would nice if dnSpy could use for loops instead of while loops
Actual Behavior
Its creating sub-optimal source
Additional Context
I get constant shade at the RimWorld discord when I promote dnSpy mainly because of issues like this. I love and will continue to use dnSpy when coding with Harmony, so this is more of a heads-up.
funkvps commented
Have you tried the new-ilspy branch? From description, it seems new version ilspy will produce for loop, than new-ilspy branch should be ok
pardeike commented
Yes, that fixes the code generation.