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

Add wrap method to remove duplicate code in effects #1470

Merged
merged 2 commits into from Mar 17, 2018

Conversation

4 participants
@default0
Contributor

default0 commented Mar 17, 2018

Adds and exposes a wrap method that removes duplicate code that was shared between EdgeEffect and BlurEffect.

This also fixes #1167.

@Tom94

We programmed this together.

@Tom94

Tom94 approved these changes Mar 17, 2018

@Tom94 Tom94 merged commit 43b4d23 into ppy:master Mar 17, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@peppy peppy added this to the March 2018 milestone Mar 17, 2018

@peppy

This comment has been minimized.

Member

peppy commented Mar 21, 2018

This broke the VolumeControl display in osu!.

peppy added a commit that referenced this pull request Mar 21, 2018

/// <summary>
/// Holds extension methods for <see cref="Container{T}"/>.
/// </summary>
public static class ContainerExtensions

This comment has been minimized.

@UselessToucan

This comment has been minimized.

@default0

default0 Mar 21, 2018

Contributor

Putting extensions in a different namespace than the thing they extend lowers their discoverability by a considerable margin. No hard feelings about where in particular this method goes myself, though.

This comment has been minimized.

@Tom94

Tom94 Mar 21, 2018

Collaborator

IIRC our Extensions folder is only for external types (e.g. Color4). Things local to the framework (such as TransformableExtensions) are indeed in the same namespace.

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