Skip to content

Update Sound-Expansion#2

Open
Andre601 wants to merge 2 commits into
PlaceholderAPI:masterfrom
Andre601:master
Open

Update Sound-Expansion#2
Andre601 wants to merge 2 commits into
PlaceholderAPI:masterfrom
Andre601:master

Conversation

@Andre601
Copy link
Copy Markdown

@Andre601 Andre601 commented Aug 4, 2021

Adds a bunch of improvements from which one is the move to a proper Maven project...

  • Made volume and pitch float numbers instead of integers (fixes The expansion has wrong code #1)
  • Updated the readme with placeholder info
  • Created a pom.xml and populated it with necessary stuff
  • Created a .gitignore to ignore the .settings, .idea and bin folder alongside PlaceholderAPI-Expansion-Sound.iml (Why are they even included?)
  • QoL code improvements (Removed pointless overrides, added missing Nonnull annotations, made certain fields final, etc)

Note that I set target and source to Java 11. This should be fine for most if not all servers and if not can I lower it to 8 when needed.
My basic testing has not shown any issues so far, but I would appreciate if you too could compile and test the expansion to see if it may have some OS dependant issues or something...

iGabyTM
iGabyTM previously requested changes Aug 4, 2021
Copy link
Copy Markdown
Member

@iGabyTM iGabyTM left a comment

Choose a reason for hiding this comment

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

Put this into a variable

@iGabyTM iGabyTM self-requested a review August 4, 2021 16:58
@Andre601
Copy link
Copy Markdown
Author

Andre601 commented Aug 4, 2021

Put this into a variable

Put what into a variable?

Copy link
Copy Markdown
Member

@iGabyTM iGabyTM left a comment

Choose a reason for hiding this comment

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

Put that Patter#quote into a variable

@iGabyTM iGabyTM dismissed their stale review August 4, 2021 18:58

Wrong

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.

The expansion has wrong code

2 participants