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

[Testing] FakeName 1.2.2 #3520

Closed
wants to merge 3 commits into from
Closed

[Testing] FakeName 1.2.2 #3520

wants to merge 3 commits into from

Conversation

ArchiDog1998
Copy link

Change your and the others' names visually.

@bleatbot
Copy link
Collaborator

bleatbot commented May 17, 2024

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!

@bleatbot bleatbot enabled auto-merge (squash) May 17, 2024 00:42
@philpax
Copy link
Contributor

philpax commented May 18, 2024

bleatbot, approve

@bleatbot
Copy link
Collaborator

bleatbot commented May 18, 2024

Outdated attempt

⚠️ No builds attempted! This probably means that your owners property is misconfigured.

👽FakeName [testing-live] - 408cf48 - Not your plugin

Show log - Review

@bleatbot bleatbot added the new plugin This is a new plugin. label May 18, 2024
@bleatbot
Copy link
Collaborator

bleatbot commented May 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!

✔️FakeName [testing-live] - 408cf48 - v1.2.0.0 - Repo (New plugin)

Show log - Review

@bleatbot bleatbot added the size-large Diff for this PR is large. label May 18, 2024
@philpax philpax added pending-code-review This plugin still needs code review. pending-rules-compliance This plugin still needs to be checked for rules compliance by the majority of the PAC. pending-testing This plugin still needs to be tested. completed-code-review Someone reviewed this. Check if the review wasn't invalidated already. and removed pending-code-review This plugin still needs code review. labels May 18, 2024
@philpax
Copy link
Contributor

philpax commented May 18, 2024

Sorry for the delay! We're taking this plugin through the review process now and will update as it goes along 🙂

EDIT: I did a code review, but other members of the team wanted to take a closer look. Will update once that's done...

@philpax philpax added pending-code-review This plugin still needs code review. and removed completed-code-review Someone reviewed this. Check if the review wasn't invalidated already. labels May 18, 2024
@ArchiDog1998 ArchiDog1998 changed the title [TESTING] FakeName 1.2.0 [Testing] FakeName 1.2.0 Jun 5, 2024
@philpax philpax added completed-rules-compliance This plugin is compliant with all rules. and removed pending-rules-compliance This plugin still needs to be checked for rules compliance by the majority of the PAC. labels Jul 10, 2024
@philpax
Copy link
Contributor

philpax commented Jul 10, 2024

Hi, sorry about how long this has all taken - we've been busy with DT prep and haven't had the chance to get through the backlog.

We're happy with this from a rules perspective, but have a few concerns:

  • Can you clarify that other players cannot see the changes you've made locally in the plugin description? This may not be obvious to non-technical users.
  • Along the same lines, can you clarify that this does not make you stream-safe as other players can still deanonymise you?
  • What steps have you taken to ensure that the changed names do not get sent to the server in any form? I trust that it's a purely visual change, but it's hard to know where data may flow from inspection of your code.

@ArchiDog1998
Copy link
Author

Hi! This plugin isn't ready for DT expansion. I am waiting for the goatcorp/Dalamud#1915 merging to make it easier to change the name on the NamePlate. Here are the answers.

  • All changes that you make using the plugin are for your client only and other people that you play with or meet in-game will still see your real in-game character name.
  • Your character's in-game name can still be tracked by other in-game players or by using other third-party resources so DON'T USE IT TO STREAM as there are people who could track and do harm to you.
  • Technically, it displays the fake name by replacing the text in SeString. These are two places to modify game data. So the changed data won't be sent to the server. And it is a purely visual change.

@philpax
Copy link
Contributor

philpax commented Jul 11, 2024

Thanks, appreciate it! Looking forward to it 🙂

@reiichi001
Copy link
Contributor

bleatbot, rebuild

@bleatbot
Copy link
Collaborator

bleatbot commented Jul 25, 2024

Outdated attempt

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

❌FakeName [testing-live] - 408cf48 - Build failed (Diff)

Show log - Review

@bleatbot bleatbot added build failed This plugin failed to build. and removed size-large Diff for this PR is large. labels Jul 25, 2024
auto-merge was automatically disabled July 25, 2024 14:58

Head branch was pushed to by a user without write access

@ArchiDog1998
Copy link
Author

Hi, this plugin is DawnTrail ready!

@bleatbot
Copy link
Collaborator

bleatbot commented Jul 25, 2024

Outdated attempt

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

✔️FakeName [testing-live] - 28fe44a - v1.2.2.0 - Repo (New plugin)

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 25, 2024
@ArchiDog1998 ArchiDog1998 changed the title [Testing] FakeName 1.2.0 [Testing] FakeName 1.2.2 Jul 26, 2024
@philpax philpax added completed-code-review Someone reviewed this. Check if the review wasn't invalidated already. and removed pending-code-review This plugin still needs code review. labels Aug 13, 2024
@philpax
Copy link
Contributor

philpax commented Aug 13, 2024

bleatbot, rebuild

@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!

✔️FakeName [testing-live] - 28fe44a - v1.2.2.0 - Repo (New plugin)

Show log - Review

@haroldiedema
Copy link
Contributor

This one is consistently silently crashing me to desktop if you give the primary player a name of the maximum amount of characters the input box allows. It does not seem to matter if a first and last name is given. There are no errors in any of the logs, and I'm not getting an exception window.

You can easily reproduce this at the character selection screen and hovering over your different characters. Feels like an Atk/UI issue.

@philpax philpax added pending-rules-compliance This plugin still needs to be checked for rules compliance by the majority of the PAC. and removed completed-rules-compliance This plugin is compliant with all rules. labels Aug 14, 2024
@philpax
Copy link
Contributor

philpax commented Aug 14, 2024

Hi; I'm very sorry about the back-and-forth on this, as well as the time it's taken to reach this conclusion.

We were initially in favour of merging this, but there were concerns raised by members of the community with regards to its potential for abuse. Based on this, and further internal discussion, we have reached the conclusion that we can't accept this plugin as its potential for abuse outweighs its positive use cases.

We apologise for this outcome and the time it took to reach it, and we hope that you're willing to submit other plugins in future.

@philpax philpax closed this Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed-code-review Someone reviewed this. Check if the review wasn't invalidated already. new plugin This is a new plugin. pending-rules-compliance This plugin still needs to be checked for rules compliance by the majority of the PAC. pending-testing This plugin still needs to be tested. size-large Diff for this PR is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants