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

PetNicknames #2084

Merged
merged 4 commits into from
Jul 29, 2023
Merged

PetNicknames #2084

merged 4 commits into from
Jul 29, 2023

Conversation

Glyceri
Copy link
Contributor

@Glyceri Glyceri commented Jul 4, 2023

[New Plugin: Pet Nicknames (0.2.0)]
[Testing] Pet Nicknames 0.2.0

Adds the ability to give nicknames to pets by using /petname
Ability to remove and disable the displaying of nicknames
Nicknames get saved based on pet

[Changelog 0.1.0 -> 0.2.0]

  • Complete plugin rewrite (now more stable)
  • Clear all pet names option
  • Disable the display of custom names option

@Glyceri
Copy link
Contributor Author

Glyceri commented Jul 4, 2023

bleatbot, rebuild

@bleatbot
Copy link
Collaborator

bleatbot commented Jul 4, 2023

Outdated attempt

This is the first time that you have submitted a plugin here. Before the bot will build your plugin within the 'Build PR' check, someone from the approval team will need to enable builds for you.

Once this is enabled, the bot will automatically build the PR. Future iterations will not require an approval for building the PR, only merging.

Please hold!

@Glyceri
Copy link
Contributor Author

Glyceri commented Jul 4, 2023

Ohhhhhhhhhhhh... I get it

@philpax philpax added the new plugin This is a new plugin. label Jul 5, 2023
@philpax
Copy link
Contributor

philpax commented Jul 5, 2023

bleatbot, approve

@bleatbot
Copy link
Collaborator

bleatbot commented Jul 5, 2023

Outdated attempt

Builds failed, please check action output.

😰PetNicknames [testing-live] - adbef5b - Build system error: DalamudPackager output not found, make sure it is installed

Show log - Review

@bleatbot bleatbot added the build failed This plugin failed to build. label Jul 5, 2023
New commit, this one probably has the build error fixed.
@bleatbot
Copy link
Collaborator

bleatbot commented Jul 5, 2023

Outdated attempt

Builds failed, please check action output.

😰PetNicknames [testing-live] - 37b12b9 - Build system error: DalamudPackager output not found, make sure it is installed

Show log - Review

@bleatbot
Copy link
Collaborator

bleatbot commented Jul 5, 2023

Outdated attempt

Builds failed, please check action output.

😰PetNicknames [testing-live] - 37b12b9 - Build system error: DalamudPackager output not found, make sure it is installed

Show log - Review

@bleatbot
Copy link
Collaborator

bleatbot commented Jul 5, 2023

Outdated attempt

Builds failed, please check action output.

😰PetNicknames [testing-live] - 37b12b9 - Build system error: DalamudPackager output not found, make sure it is installed

Show log - Review

@bleatbot
Copy link
Collaborator

bleatbot commented Jul 5, 2023

All builds OK!

✔️PetRenamer [testing-live] - 926fd51 - v0.2.0.0 - Diff (1172 lines)

Show log - Review

@bleatbot bleatbot added size-large Diff for this PR is large. and removed build failed This plugin failed to build. labels Jul 5, 2023
@reiichi001
Copy link
Contributor

Hello! Going through all plugins for review/testing.

I have a couple notes on this plugin that aren't blockers, but could be handy to address before release.

  1. I would add a note that "pet" in this case means Minions, as FFXIV uses "Pet" to refer to things like Scholar/Summoner pets like Eos, Selene, Carbuncles, and Egis. The plugin doesn't allow renaming those.
  2. I would include a note in the description and/or config box that the name change will take place on zone change or by re-summoning the pet. Or if you know how, it could help to redraw the pet on name change / add a button to unsummon/resummon it to apply.

@Glyceri
Copy link
Contributor Author

Glyceri commented Jul 14, 2023

I implemented the feedback as requested, would it be benefitial to push that for testing right now?
(You can also look away from a minion and look back and the name updates, weirdly enough any other interference like me moving the minion or forcing a respawn does not work, but when I have a sub again ill look into it).

@philpax philpax enabled auto-merge (squash) July 29, 2023 19:35
@philpax philpax merged commit 13a91ba into goatcorp:main Jul 29, 2023
2 checks passed
@philpax
Copy link
Contributor

philpax commented Jul 29, 2023

Congratulations on your first plugin!

As a first-time contributor, if you're in the XIVLauncher & Dalamud Discord server and would like to get access to the private developer channels for plugin feedback and others, please let us know your Discord handle and we'll grant you the role.


I reviewed the code at the most recent GitHub commit - you'll probably want to push up a follow-up PR with the latest update! Promise it'll be faster this time :)

@Glyceri Glyceri deleted the PetNicknames-0.2.0 branch July 30, 2023 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new plugin This is a new plugin. size-large Diff for this PR is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants