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

Update Tf2CriticalHitsPlugin for FFXIV 7.01 #4053

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

jkchen2
Copy link
Contributor

@jkchen2 jkchen2 commented Jul 18, 2024

This is a request to adopt the Tf2CriticalHitsPlugin, as the original author (Berna) has decided to stop development. I don't intend to do feature development, but at the very least wanted to get this plugin working on Dawntrail.

Berna's message on Discord regarding stopping development and leaving their plugins up for adoption: https://discord.com/channels/581875019861328007/1069307522269319178/1259198126376488960

Berna's message on GitHub in response to my request to have the plugin be adopted: Berna-L/ffxiv-tf2-crit-plugin#7 (comment)

The bulk of the changes required for the update were compatibility changes for API 10. There are also a few smaller changes to how certain sound logic is handled (using FFXIVClientStructs for built-in sound playing instead of doing it in-house). I've done a fair amount of testing to confirm the behavior works as expected for both the crit sound effects, as well as the Countdown Jams feature, though admittedly, it's not 100% comprehensive.

Additionally, I've elected to keep Berna as the author, as I only intend to keep this plugin compatible with FFXIV/Dalamud updates. If this needs to change, please let me know.

@bleatbot bleatbot enabled auto-merge (squash) July 18, 2024 06:04
@reiichi001
Copy link
Contributor

bleatbot, approve

@bleatbot
Copy link
Collaborator

bleatbot commented Jul 18, 2024

Outdated attempt

Take care! Please test your plugins in-game before submitting them here to prevent crashes and instability. We really appreciate it!

The average merge time for plugin updates is currently 5 hours.

Name Commit Status
Tf2CriticalHitsPlugin [stable] 3c35b6b Build failed (Diff)
Tf2CriticalHitsPlugin [testing-live] 3c35b6b Build failed (Diff)
Show log - Review

@bleatbot bleatbot added the build failed This plugin failed to build. label Jul 18, 2024
@jkchen2
Copy link
Contributor Author

jkchen2 commented Jul 18, 2024

If I'm reading this right, it seems the build failure may be caused by the existence of the submodule? If so, should I just fold the files from the submodule directly into the plugin?

Edit: I think there's a possibility that this error was this: goatcorp/Plogon#61
Doesn't look to be reproducible, so I'm running a rebuild to see.

@jkchen2
Copy link
Contributor Author

jkchen2 commented Jul 18, 2024

bleatbot, rebuild

@bleatbot
Copy link
Collaborator

bleatbot commented Jul 18, 2024

Outdated attempt

Take care! Please test your plugins in-game before submitting them here to prevent crashes and instability. We really appreciate it!

Name Commit Status
Tf2CriticalHitsPlugin [stable] 3c35b6b Build failed (Diff)
Tf2CriticalHitsPlugin [testing-live] 3c35b6b Build failed (Diff)
Show log - Review

auto-merge was automatically disabled July 18, 2024 08:16

Head branch was pushed to by a user without write access

@bleatbot
Copy link
Collaborator

Take care! Please test your plugins in-game before submitting them here to prevent crashes and instability. We really appreciate it!

Name Commit Status
✔️ Tf2CriticalHitsPlugin [stable] b6119d2 v3.5.0.0 - Diff (72 lines, prev. 3.4.1.0) - Semantic
✔️ Tf2CriticalHitsPlugin [testing-live] b6119d2 v3.5.0.0 - Diff (128 lines, prev. 3.4.0.0) - Semantic
Show log - Review

@bleatbot bleatbot added size-small Diff for this PR is small. and removed build failed This plugin failed to build. labels Jul 18, 2024
@jkchen2
Copy link
Contributor Author

jkchen2 commented Jul 18, 2024

Yeah, looks like the git command in the csproj was the problem.

@philpax philpax enabled auto-merge (squash) July 19, 2024 17:35
@philpax philpax merged commit 168239a into goatcorp:main Jul 19, 2024
2 checks passed
@philpax
Copy link
Contributor

philpax commented Jul 19, 2024

Congratulations on adopting a plugin and making your first contribution to the ecosystem!

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.

@jkchen2
Copy link
Contributor Author

jkchen2 commented Jul 20, 2024

@philpax Thanks! I'll take you up on that offer for the dev channels -- I'm @trash.cat on Discord.

@jkchen2 jkchen2 mentioned this pull request Jul 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size-small Diff for this PR is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants