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

Some Features/Fixes #28

Merged
merged 20 commits into from
Oct 2, 2023
Merged

Some Features/Fixes #28

merged 20 commits into from
Oct 2, 2023

Conversation

IRacle1
Copy link
Contributor

@IRacle1 IRacle1 commented Oct 1, 2023

  • Some code refactor and fix nullable warning
  • IReadOnlyCollection instead of IEnumerable in LocalLevels
  • Optimized MultiTrigger property for Touch and Spawn triggers (also idk about IgnoreField, i think it can be deleted)
  • Added GameModeratorType for readability in Account class

I also changed logic in typedescriptor, GetPropertiesAndFields, for old logic fields in inherited classes werent work properly, the logic in my pr works fine for that, but I still could be wrong

@IRacle1
Copy link
Contributor Author

IRacle1 commented Oct 1, 2023

All tests passed except for game resources, so alright
image

@Folleach
Copy link
Owner

Folleach commented Oct 1, 2023

Thank you for your contribution!
I'll check this in the nearest time

For now I've noticed that you've used the dev branch. This branch is outdated for 4 months.
But! It's okay, I'll add your branch to the main one eventually.

Maybe I should write to contribution.md about which branches it's worth creating your own features from

# Conflicts:
#	GeometryDashAPI/Serialization/TypeDescriptor.cs
#	GeometryDashAPI/Server/Dtos/Account.cs
#	GeometryDashAPI/Server/GameClient.cs
because TypeDescriptor can cast enums itself
Comment on lines 237 to 238
foreach (var field in type.GetFields(BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public))
yield return field;
Copy link
Owner

@Folleach Folleach Oct 2, 2023

Choose a reason for hiding this comment

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

I've rollback this, because GetFields doesn't return private fields from inherited classes
Also added test for this TypeDescriptorTests.PrivateFieldFromInheritedClass

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, i see, i thought private field form base class can be found using GetFieds(NonPublic), my mistake.

Copy link
Owner

Choose a reason for hiding this comment

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

It’s ok, I thought too at first time xd

becuase GetFields doesn't return private fields from inherited classes
@Folleach
Copy link
Owner

Folleach commented Oct 2, 2023

I've finished
If you don't have any questions, we can release this!

@IRacle1
Copy link
Contributor Author

IRacle1 commented Oct 2, 2023

I've finished If you don't have any questions, we can release this!

Yeah, tnx for your changes, now all looks great. i also changed linq expression by a usefull flag, should work faster

@Folleach
Copy link
Owner

Folleach commented Oct 2, 2023

i also changed linq expression by a usefull flag, should work faster

Oh, great!
I didn't notice this flag

You're right about performance, it's almost 2x faster!

Method Mean Error StdDev Median Gen0 Allocated
BindingFlag 69.17 ns 1.817 ns 5.330 ns 67.35 ns 0.0305 64 B
Linq 108.19 ns 2.207 ns 4.407 ns 106.65 ns 0.0421 88 B
class A
{
    protected int X;
}

class B : A
{
    protected int Y;
}

[MemoryDiagnoser]
public class GetFieldsBenchmarks
{
    private Consumer c = new Consumer();

    [Benchmark]
    public void BindingFlag()
    {
        typeof(B).GetFields(BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly).Consume(c);
    }

    [Benchmark]
    public void Linq()
    {
        typeof(B).GetFields(BindingFlags.NonPublic | BindingFlags.Instance).Where(x => x.DeclaringType == typeof(B)).Consume(c);
    }
}

@Folleach Folleach merged commit aac1bb1 into Folleach:dev Oct 2, 2023
@Folleach
Copy link
Owner

Folleach commented Oct 2, 2023

Released in v0.2.19

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

Successfully merging this pull request may close these issues.

2 participants