falkirks / Specter

Super simple debugging for PocketMine.
MIT License
73 stars 35 forks source link

Compatibility with Alpha9 #60

Closed 95CivicSi closed 7 years ago

95CivicSi commented 7 years ago

This update breaks BC.

falkirks commented 7 years ago

That hardcoding is really ugly. Is there no better way to get a default skin?

95CivicSi commented 7 years ago

It would be possible to move the skin data to a config file and pull from there. It's also possible to load the skin from an existing player if they are the one who uses the command. I'll have to check other methods.

SOF3 commented 7 years ago

Also note a potential copyright infringement by embedding it in the plugin.

95CivicSi commented 7 years ago

@SOF3 I imagine you or @dktapps would be more knowledgeable about loading a default skin with the new API. I hadn't considered the copyright problem. If push comes to shove, we could load a custom skin.

dktapps commented 7 years ago

There's no need for an original skin. The original grey Specter one works fine, it just needs to be base64 encoded when assigned to SkinData. Also, only the SkinId and SkinData fields are required.

dktapps commented 7 years ago

Also note that verification skip is not supported on the release build of ALPHA9, only latest development builds.

95CivicSi commented 7 years ago

@dktapps Thanks for the replies. I had not used Specter prior to this and did not realize that the previous skin data was just generating a gray color. That made a huge difference in understanding how to change this. @Falkirks Commit https://github.com/Falkirks/Specter/pull/60/commits/c96e97801bf91c42073e80278219b8b753dfc196 on my branch reverts skin data to the previous setting with one change. SkinID is now "Specter" to help eliminate future misunderstandings.

image