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

Moved more traits to Mods.Common #7570

Merged
merged 1 commit into from
Mar 15, 2015
Merged

Conversation

Mailaender
Copy link
Member

Tried to see how far I could come regarding #7526.

@Mailaender Mailaender force-pushed the mods-common branch 2 times, most recently from 5ca401e to ebcc8f8 Compare March 1, 2015 16:39
@@ -67,8 +67,6 @@ Chrome:
Assemblies:
./mods/common/OpenRA.Mods.Common.dll
./mods/ra/OpenRA.Mods.RA.dll
./mods/cnc/OpenRA.Mods.Cnc.dll
Copy link
Member

Choose a reason for hiding this comment

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

This will break the custom RA maps that port the Ion Cannon and other TD things into RA.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are there any?

Copy link
Member

Choose a reason for hiding this comment

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

There are quite a few on the resource center, unfortunately.

Copy link
Member Author

Choose a reason for hiding this comment

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

So keep that DLL or move the Ion Cannon to Mods.Common?

Copy link
Member

Choose a reason for hiding this comment

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

My preference would be to keep the DLL for now, and then later replace the overly specific Ion Cannon and Nuke support powers with a more general "Fire an armament" power in mods.common (with their special missile / beam behaviour implemented as projectiles).

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

@obrakmann
Copy link
Contributor

Looks ok to me, didn't notice any regression in-game. 👍

Needs a rebase.

@Mailaender
Copy link
Member Author

Rebased.

namespace OpenRA.Mods.RA.Scripting
{
[ScriptPropertyGroup("Paradrop")]
public class ParadropPowers : ScriptActorProperties, Requires<CargoInfo>, Requires<ParaDropInfo>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with how the Lua API works, but shouldn't this be called ParadropProperties ?
Either that or the file name needs to be changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with penev here, please rename for consistency, and then let's get this merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry. Forgot about this one. Updated.

@reaperrr
Copy link
Contributor

Looks good and noticed no issues on RA and TD shellmaps. 👍

reaperrr added a commit that referenced this pull request Mar 15, 2015
Moved more traits to Mods.Common
@reaperrr reaperrr merged commit a14bf86 into OpenRA:bleed Mar 15, 2015
@Mailaender Mailaender deleted the mods-common branch March 16, 2015 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants