Civcraft / NameLayer

Do not open issues here; open them on the maintained fork @ DevotedMC
https://github.com/DevotedMC/NameLayer
BSD 3-Clause "New" or "Revised" License
4 stars 14 forks source link

Convert NMS to reflection #203

Closed goeppes closed 8 years ago

goeppes commented 8 years ago

Currently NameLayer uses a combination of NMS and reflection to change a player's name. However, it is possible to do this using only reflection and no NMS.

Removing the NMS dependency would:

  1. make it so that we do not need to create a new subproject for each new version of Minecraft.
  2. allow us to remove all of the NMS subprojects from the repository, making it easier for new developers to find their way around.
  3. allow us to remove the ridiculous circular dependency where the subprojects depend on the main project and the main project depends on the subprojects.

Is there an interest in having this done, or is there an issue with using only reflection that I am unaware of (as opposed to using both NMS and reflection)?

rourke750 commented 8 years ago

How could you use reflection without referencing the objects required to change them?

goeppes commented 8 years ago

By obtaining their classes and instances using reflection. More code will be needed, but I think the benefit of no longer having to create a new subproject with each version outweighs that.

Here is an example in an incomplete plugin I've written.

ProgrammerDan commented 8 years ago

I've seen both ways used to great success in projects, and honestly I find the subproject approach like NameLayer, NoCheatPlus, Orebfuscator and others use to be, ultimately, clearer and cleaner then the pure reflective approach.

As well we can probably do better on the dependencies issue within the subprojects here, that's never sat well with me. I'll look into it.

rourke750 commented 8 years ago

My thing with the current system is it does allow people to build NameLayer without needing the nmS stuff. On Jul 1, 2016 10:42 PM, "Daniel Boston" notifications@github.com wrote:

I've seen both ways used to great success in projects, and honestly I find the subproject approach like NameLayer, NoCheatPlus, Orebfuscator and others use to be, ultimately, clearer and cleaner then the pure reflective approach.

As well we can probably do better on the dependencies issue within the subprojects here, that's never sat well with me. I'll look into it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Civcraft/NameLayer/issues/203#issuecomment-230079513, or mute the thread https://github.com/notifications/unsubscribe/ABkpKEWZaGVFDZRrxUXWrq355zHtAuRZks5qRc_3gaJpZM4JDk_o .

rourke750 commented 8 years ago

Either way is fine but I thing we should keep the modules regardless On Jul 1, 2016 10:47 PM, "Rourke750" rourke7501@gmail.com wrote:

My thing with the current system is it does allow people to build NameLayer without needing the nmS stuff. On Jul 1, 2016 10:42 PM, "Daniel Boston" notifications@github.com wrote:

I've seen both ways used to great success in projects, and honestly I find the subproject approach like NameLayer, NoCheatPlus, Orebfuscator and others use to be, ultimately, clearer and cleaner then the pure reflective approach.

As well we can probably do better on the dependencies issue within the subprojects here, that's never sat well with me. I'll look into it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Civcraft/NameLayer/issues/203#issuecomment-230079513, or mute the thread https://github.com/notifications/unsubscribe/ABkpKEWZaGVFDZRrxUXWrq355zHtAuRZks5qRc_3gaJpZM4JDk_o .

goeppes commented 8 years ago

Hardly anyone looks at the subprojects, so I think sacrificing clarity in exchange for no longer having to look at that code at all makes sense. If you guys would like, I can throw something together and make a pull request so you can better compare.

In the event that it is decided the subproject method is preferred, I think that at the very least it would be good to select the GameProfile field from the EntityHuman class using pure reflection instead of looking up the obfuscated field name and hardcoding it into each version.