Skip to content
This repository has been archived by the owner on Aug 15, 2022. It is now read-only.

Added Unity assembly definitions #287

Closed
wants to merge 1 commit into from
Closed

Added Unity assembly definitions #287

wants to merge 1 commit into from

Conversation

JasperCiti
Copy link

@JasperCiti JasperCiti commented Jun 23, 2019

@phalasz
Copy link
Contributor

phalasz commented Jun 23, 2019

The main issue with asmdefs is that Forge uses a lot of partial classes. So in case someone wants to add a new partial class to add more functionality to one of the Forge classes then they will have to add the file into the forge directory as those files will need to be compiled into the same assembly.

Also Forge works fine with Unity 2019, hence the title of the PR is a bit missleading.

@JasperCiti JasperCiti changed the title Added Unity assembly definitions for compatibility with Unity 2019. Added Unity assembly definitions Jun 23, 2019
@JasperCiti
Copy link
Author

That's too bad. I have try to move the generated stuff outside the assembly, but then the Chat module break. Will have to refactor the Chat module first or something...

@phalasz
Copy link
Contributor

phalasz commented Jun 23, 2019

It's not just the generated folder though, but any other partial class. As the other part of some of the generated folder's partial classes are in the main source files in Scripts/Networking.

@JasperCiti
Copy link
Author

JasperCiti commented Jun 23, 2019

But if you have code inside one of your own assemblies that depend on FNR, how do you reference that?

@phalasz
Copy link
Contributor

phalasz commented Jun 24, 2019

You can reference them by adding FNR as a dependency to your assembly. The main issue is if you want to add further functionality to a partial class, as that would need to be done in the FNR directories so it gets added to the FNR assembly.

c# docs:

All partial-type definitions meant to be parts of the same type must be defined in 
the same assembly and the same module (.exe or .dll file). Partial definitions cannot 
span multiple modules.

@phalasz
Copy link
Contributor

phalasz commented Jun 24, 2019

One option could be is to move Assets/Bearded Man Studios Inc/ForgeNetworkingRemaster.asmdef into Assets/Bearded Man Studios Inc/Scripts/Networking/ as that folder should not have any partial classes and used to be compiled into a dll in the past.

@BrentFarris
Copy link
Member

I have merged a big cleanup branch and so you will probably need to merge current develop into your fork and resolve conflicts, sorry about this. The main breaking change in develop is the folder paths have updated their names to not have spaces. Please see this commit for more info on the changes

Copy link
Member

@Crazy8ball Crazy8ball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes per comment above.

@phalasz
Copy link
Contributor

phalasz commented Mar 23, 2020

Closing this as there have been no movement on it for a long time.

@phalasz phalasz closed this Mar 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants