-
Notifications
You must be signed in to change notification settings - Fork 42
Condition Parameter Generation #567
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
Changes from all commits
410458c
57466cf
8c53eac
d6bb1e3
d481ad6
6a1b84c
eba9a24
30bd828
10f6c0b
e06cdb6
039f6f1
c6df52c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| using Loqui.Generation; | ||
| using Noggog; | ||
| using Noggog.StructuredStrings; | ||
| using Noggog.StructuredStrings.CSharp; | ||
| using ObjectType = Mutagen.Bethesda.Plugins.Meta.ObjectType; | ||
| namespace Mutagen.Bethesda.Generation.Modules.Plugin; | ||
|
|
||
| public class ConditionDataModule : GenerationModule | ||
| { | ||
| public override Task PreLoad(ObjectGeneration obj) | ||
| { | ||
| var data = obj.GetObjectData(); | ||
| data.GenerateConditionData = obj.Node.GetAttribute("generateConditionData", defaultVal: true); | ||
| return base.PreLoad(obj); | ||
| } | ||
|
|
||
| public override async Task GenerateInClass(ObjectGeneration obj, StructuredStringBuilder sb) | ||
| { | ||
| await base.GenerateInClass(obj, sb); | ||
|
|
||
| if (obj.Abstract) return; | ||
| if (obj.GetObjectType() != ObjectType.Subrecord) return; | ||
| if (obj.BaseClass is null) return; | ||
| if (obj.BaseClassName != "ConditionData") return; | ||
| if (obj.GetObjectData().GenerateConditionData == false) return; | ||
|
|
||
| var i = 1; | ||
| foreach (var field in obj.Fields) | ||
| { | ||
| if (i > 2) break; | ||
| if (field.Name.Contains("UnusedStringParameter")) continue; | ||
| var isUnusedParameter = field.Name.Contains("Unused"); | ||
|
|
||
| // Parameter property | ||
| sb.AppendLine($"public override object? Parameter{i}"); | ||
| using (sb.CurlyBrace()) | ||
| { | ||
| if (isUnusedParameter) | ||
| { | ||
| sb.AppendLine("get => null;"); | ||
| sb.AppendLine("set"); | ||
| using (sb.CurlyBrace()) | ||
| { | ||
| sb.AppendLine(); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| var name = field.Name; | ||
| var setterTypeName = field.TypeName(false); | ||
| sb.AppendLine($"get => {name};"); | ||
| sb.AppendLine($"set => {name} = (value is {setterTypeName} v ? v : throw new ArgumentException());"); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| // ParameterType property | ||
| sb.AppendLine($"public override Type? Parameter{i}Type"); | ||
| using (sb.CurlyBrace()) | ||
| { | ||
| if (isUnusedParameter) | ||
| { | ||
| sb.AppendLine("get => null;"); | ||
| } | ||
| else | ||
| { | ||
| var name = field.TypeName(true); | ||
| sb.AppendLine($"get => typeof({name});"); | ||
| } | ||
| } | ||
|
|
||
| i++; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,7 +118,7 @@ public Mask(TItem initialValue) | |
| public Mask( | ||
| TItem RunOnType, | ||
| TItem Reference, | ||
| TItem Unknown3, | ||
| TItem RunOnTypeIndex, | ||
| TItem UseAliases, | ||
| TItem UsePackageData, | ||
| TItem FirstUnusedIntParameter, | ||
|
|
@@ -128,7 +128,7 @@ public Mask( | |
| : base( | ||
| RunOnType: RunOnType, | ||
| Reference: Reference, | ||
| Unknown3: Unknown3, | ||
| RunOnTypeIndex: RunOnTypeIndex, | ||
| UseAliases: UseAliases, | ||
| UsePackageData: UsePackageData) | ||
| { | ||
|
|
@@ -454,6 +454,33 @@ public static implicit operator TranslationMask(bool defaultOn) | |
| } | ||
| #endregion | ||
|
|
||
| #region Mutagen | ||
| public override object? Parameter1 | ||
| { | ||
| get => null; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Getting null here feels fine. Feels a bit odd to just do nothing in the set. Gut is telling me we should throw if the given value is not null (similar to how we throw if the type is wrong for used parameters)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was worrying this could lead to strange behavior when you are working on generic conditions. Setting a property might not be intuitively something where you expect it throws when doing it? What do you think? You can of course just add a try catch block when you get any issues with this.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, not a great way either route... but I think that's just the price of writing generic API that is trying to handle everything. The downside of the current setup is that users will expect that if they set the field and it "went through", that something was modified. But instead, it didnt modify anything, and was just a "silent error" of sorts. The throw at least is something the user can detect/handle, vs the silent error just leads to more of an undetectable paranoia, imo |
||
| set | ||
| { | ||
|
|
||
| } | ||
| } | ||
| public override Type? Parameter1Type | ||
| { | ||
| get => null; | ||
| } | ||
| public override object? Parameter2 | ||
| { | ||
| get => null; | ||
| set | ||
| { | ||
|
|
||
| } | ||
| } | ||
| public override Type? Parameter2Type | ||
| { | ||
| get => null; | ||
| } | ||
| #endregion | ||
|
|
||
| #region Binary Translation | ||
| [DebuggerBrowsable(DebuggerBrowsableState.Never)] | ||
| protected override object BinaryWriteTranslator => CanFlyHereConditionDataBinaryWriteTranslation.Instance; | ||
|
|
@@ -677,7 +704,7 @@ internal enum CanFlyHereConditionData_FieldIndex | |
| { | ||
| RunOnType = 0, | ||
| Reference = 1, | ||
| Unknown3 = 2, | ||
| RunOnTypeIndex = 2, | ||
| UseAliases = 3, | ||
| UsePackageData = 4, | ||
| FirstUnusedIntParameter = 5, | ||
|
|
@@ -914,7 +941,7 @@ public static CanFlyHereConditionData_FieldIndex ConvertFieldIndex(ConditionData | |
| return (CanFlyHereConditionData_FieldIndex)((int)index); | ||
| case ConditionData_FieldIndex.Reference: | ||
| return (CanFlyHereConditionData_FieldIndex)((int)index); | ||
| case ConditionData_FieldIndex.Unknown3: | ||
| case ConditionData_FieldIndex.RunOnTypeIndex: | ||
| return (CanFlyHereConditionData_FieldIndex)((int)index); | ||
| case ConditionData_FieldIndex.UseAliases: | ||
| return (CanFlyHereConditionData_FieldIndex)((int)index); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting it here too, but we shuld check if it's not null and throw argument exception if it's not null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a similar design decision as throwing the other exception. Should be both or none.