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

Workflow improvement - Delete Bindings #53

Open
AustinSmith13 opened this issue Jan 19, 2020 · 2 comments
Open

Workflow improvement - Delete Bindings #53

AustinSmith13 opened this issue Jan 19, 2020 · 2 comments
Assignees

Comments

@AustinSmith13
Copy link
Contributor

The Issue

When you create a new Abstract type for example AbstractBaseBallScript and you generate the bindings you get BaseBallScript In your Bindings.cs. Switching back to unity editor triggers a compilation and no errors. Good.

If you add some more abstract functions to your AbstractBaseBallScript and switch back to unity editor to re-build your bindings. Unity Editor will have some Errors for you, but this does not prevent you from clicking on Generate Bindings. If you do generate bindings again GenerateBindings is accessing an older assembly without your changes to AbstractBaseBallScript. Thus Bindings.cs will not have your new changes and will continue to give compilation errors.

The only way to fix this is to manually clear out your Bindings.cs , compile your code with no errors, then click Generate Bindings. This results with bindings that reflect your changes.

One Potential Fix

The way I have fixed this currently is by adding a new editor option NativeScript/Delete Bindings. I call InjectBuilders and have them clear out the bindings for me. Now I can delete the bindings and compile without errors. One problem that still persists is you still have to fix any other errors from referencing the bindings generated class from your code-base. In my case its just a factory function.

I'm not sure if this is an already solved problem. I would like to know what your thoughts are on this.

If you think the Delete Bindings is a good idea I can create another pull request as well.

@jacksondunstan
Copy link
Owner

Thank you for taking the time to write this up in such detail, and for the offer to create a PR to improve the project!

There are three ways that you can do this right now, but none is ideal:

Manual Editing

This approach solves the problem by manually editing the bindings.

To add a new abstract method, property, or event:

  1. Add public abstract void Foo(); (for example) to AbstractBaseBallScript
  2. Double-click the error in the Unity editor to open BaseBallScript in Bindings.cs
  3. Add public override void Foo() {}

To remove an existing abstract method, property, or event:

  1. Remove public abstract void Foo(); (for example) from AbstractBaseBallScript
  2. Double-click the error in the Unity editor to open BaseBallScript in Bindings.cs
  3. Remove public override void Foo() {}

Temporary Virtual

This approach solves the problem by temporarily making the abstract method, property or event virtual so overriding is optional.

To add a new abstract method, property, or event:

  1. Add public virtual void Foo() {} (for example) to AbstractBaseBallScript
  2. Add an override to the JSON config object for the object in the BaseTypes array:
{
	"Name": "MyGame.AbstractBaseBallScript",
	"BaseTypes": [
		{
			"BaseName": "MyGame.BaseBallScript",
			"DerivedName": "MyGame.BallScript",
			"OverrideMethods": [
				{
					"Name": "Foo",
					"ParamTypes": []
				}
			]
		}
	]
}
  1. Generate bindings
  2. Change public virtual void Foo() {} to public abstract void Foo(); in AbstractBaseBallScript
  3. Remove the override from OverrideMethods in the JSON config

To remove an existing abstract method, property, or event:

  1. Change public abstract void Foo(); to public virtual void Foo() {} in AbstractBaseBallScript
  2. Generate bindings
  3. Remove public virtual void Foo() {}

Delete and Re-create

This approach solves the problem by removing the bindings, making changes, then re-creating the bindings.

To add a new abstract method, property, or event:

  1. Remove all mention of AbstractBaseBallScript from the JSON config
  2. Generate bindings
  3. Add public abstract void Foo(); (for example) to AbstractBaseBallScript
  4. Restore AbstractBaseBallScript by undoing step 1 in the JSON config
  5. Generate bindings

To remove an existing abstract method, property, or event:

  1. Remove all mention of AbstractBaseBallScript from the JSON config
  2. Generate bindings
  3. Remove public abstract void Foo(); (for example) from AbstractBaseBallScript
  4. Restore AbstractBaseBallScript by undoing step 1 in the JSON config
  5. Generate bindings

Comparison

This "Manual Editing" approach requires manual editing and temporarily shows a compiler error, but it's extremely quick to do and always guided by double-clicking the compile error.

The "Temporary Virtual" approach requires no manual editing and never produces a compiler error. It does, however, involve more steps and a fair amount of typing into the JSON config for the addition part. These steps aren't guided by compiler errors and are likely harder to figure out for most users.

The "Delete and Re-create" approach is simple and straightforward, but requires the user to perform the steps in the right order since they will not work (as you point out) after the compiler error is already present. It's not quite as guided as the "Manual Editing" approach.

A New Way?

Your "Delete Bindings" approach seems closest to the "Delete and Re-create" approach, except that it deletes all bindings. That's simpler, but may cause errors for code relying on other bindings being present. If that's the case, deleting all bindings will cause compiler errors and, ironically, prevent generating bindings.

Let me know if you have any ideas for other approaches. I'll try to think of some, too.

@AustinSmith13
Copy link
Contributor Author

AustinSmith13 commented Feb 7, 2020

I have an idea but it makes the problem worse, but I'll post it anyways because maybe it might spark other ideas.

Mono.CecilX construct dll's instead of generating readable code (Weaving). Just an idea, makes the problem worse as you can not read anything...

--Edit--
Actually I think it could work if you never directly reference anything and instead leave that up to the weaver to resolve those references and have the dll output part of the build pipeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants