Synchronizing dumper with monomod and fixing ILLabel dumping #118
Conversation
| ImplAttributes = MethodImplAttributes.IL | MethodImplAttributes.Managed, | ||
| DeclaringType = td, | ||
| HasThis = false | ||
| HasThis = false, | ||
| NoInlining = true |
There was a problem hiding this comment.
Dumps are broken without this when you try to dump methods that are inlined, or when dumping a transpiler that is postfixed, or situations like that. Same thing for relinker below. At least this is what this PR is supposed to fix, and it did when I tested it.
For context, this is what happened when I tried to dump a postfix that was inlined:
Original method:
public static NoteLineLayer GetNoteLineLayer(int lineLayer)
{
switch (lineLayer)
{
case 0:
return NoteLineLayer.Base;
case 1:
return NoteLineLayer.Upper;
case 2:
return NoteLineLayer.Top;
default:
return NoteLineLayer.Base;
}
}Postfix:
[HarmonyPatch(typeof(BeatmapDataLoaderVersion3.BeatmapDataLoader.ObstacleConverter), nameof(BeatmapDataLoaderVersion3.BeatmapDataLoader.ObstacleConverter.GetNoteLineLayer))]
internal class BeatmapDataLoaderObstacleConverterGetNoteLineLayerPatch
{
private static void Postfix(ref NoteLineLayer __result, int lineLayer)
{
if (lineLayer > 2)
{
__result = (NoteLineLayer)lineLayer;
}
}
}| Relinker relinker = (mtp, _) => | ||
| { | ||
| if (mtp == md) | ||
| return clone!; | ||
| if (mtp is MethodReference mr) | ||
| { | ||
| if (mr.FullName == md.FullName | ||
| && mr.DeclaringType.FullName == md.DeclaringType.FullName | ||
| && mr.DeclaringType.Scope.Name == md.DeclaringType.Scope.Name) | ||
| return clone!; | ||
| } | ||
| return module.ImportReference(mtp); | ||
| }; |
There was a problem hiding this comment.
Updated from reorg: https://github.com/MonoMod/MonoMod/blob/26a0a4d22fcd233dd22db4fd034cb3ac41420fd4/src/MonoMod.Utils/DMDGenerators/DMDCecilGenerator.cs#L72-L83
Which is also the same for the double Public, and everything else but the label stuff. Label stuff specifically addressed the issue I mentioned above.
|
While it fixed some of the cases I encountered previously, it seems to still be causing issues.
Patch: private static IEnumerable<CodeInstruction> Transpiler(IEnumerable<CodeInstruction> instructions)
{
// Replaces the obstacle width.
return new CodeMatcher(instructions)
.MatchEndForward(
new CodeMatch(i => i.opcode == OpCodes.Callvirt && ((MethodBase)i.operand).Name == $"get_{nameof(ObstacleController.width)}"),
new CodeMatch(OpCodes.Conv_R4),
new CodeMatch())
.ThrowIfInvalid()
.Insert(Transpilers.EmitDelegate<Func<float, float>>(obstacleWidth =>
{
if (!Plugin.active || obstacleWidth is < 1000 and > -1000)
{
return obstacleWidth;
}
if (obstacleWidth <= -1000)
{
obstacleWidth += 2000;
}
return (obstacleWidth - 1000) / 1000;
}))
.InstructionEnumeration();
}Original: public ObstacleSpawnData GetObstacleSpawnData(ObstacleData obstacleData)
{
Vector3 obstacleOffset = this.GetObstacleOffset(obstacleData.lineIndex, obstacleData.lineLayer);
obstacleOffset.y += this._jumpOffsetYProvider.jumpOffsetY;
obstacleOffset.y = Mathf.Max(obstacleOffset.y, this._verticalObstaclePosY);
float num = Mathf.Min((float)obstacleData.height * StaticBeatmapObjectSpawnMovementData.layerHeight, this._obstacleTopPosY - obstacleOffset.y);
float num2 = (float)obstacleData.width * 0.6f;
obstacleOffset.x += (num2 - 0.6f) * 0.5f;
return new ObstacleSpawnData(obstacleOffset, num2, num);
} |
|
@Meivyn If you wrote unit tests demonstrating your bug, that would be exceedingly useful and would significantly increase the likelihood that fixes get merged. |
|
It's actually because of This PR was opened after a conversation with them that started here: https://discord.com/channels/623153565053222947/1128639196169510942/1265164603151618129 |
kohanis
changed the title
|
Split into commits. Reworked fix a bit. Added a test. Left comments because they are in the original and have a contextual meaning |
|
CI failed on setup-dotnet, huh) |
No description provided.