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

Nation deletion message is broadcast after cancelling PreDeleteNationEvent #7380

Closed
galacticwarrior9 opened this issue Apr 25, 2024 · 0 comments · Fixed by #7383
Closed

Nation deletion message is broadcast after cancelling PreDeleteNationEvent #7380

galacticwarrior9 opened this issue Apr 25, 2024 · 0 comments · Fixed by #7383
Assignees
Milestone

Comments

@galacticwarrior9
Copy link
Contributor

What steps will reproduce the problem?

  1. Cancel PreDeleteNationEvent
    @EventHandler
    public void onPreNationDelete(PreDeleteNationEvent event) {
            event.setCancelled(true);
    }
  1. Observe how the nation deletion message is broadcast in chat despite the nation not being deleted.

What is the expected output?

No deletion message is broadcast.

Towny version

0.100.2.4 and 0.100.2.5

Server version

git-Purpur-2169 (MC: 1.20.4)

Please use Pastebin.com to link the following files

  1. Your full server startup from the logs\latest.log: See Line 950 onwards.
  2. Your Towny config.yml: Same as previously
  3. Your log's error: No error.
LlmDl added a commit that referenced this issue Apr 25, 2024
cancelled.

Not really happy with the implementation of this, as it changes our
DataSource pattern to one where only a single removeObject method
returns a boolean. The TownUtil change is also not ideal now or before
this PR as the Nation can be prevented from being deleted, while still
processing a refund.

Potential alternative solutions:
- Run the message in the DatabaseHandler method, and lose the custom
deleted by admin message sent to the admin who deleted the nation.
- Run the PreDeleteNation event before the removeNation(Nation) in order
to prevent the TownUtil method being made redundant, and causing issues.

Closes #7380.
@LlmDl LlmDl self-assigned this Apr 25, 2024
@LlmDl LlmDl added this to the 0.100.3.0 milestone Apr 25, 2024
LlmDl added a commit that referenced this issue Apr 26, 2024
iscancelled.

Uses the nation.exists to determine if the removal of the nation was
successful or not, and displays messages to match the outcome.

Closes #7380.
Supercedes #7381.
LlmDl added a commit that referenced this issue Apr 26, 2024
…celled. (#7383)

* Fix nation deleted message showing when a predeletenationevent
iscancelled.

Uses the nation.exists to determine if the removal of the nation was
successful or not, and displays messages to match the outcome.

Closes #7380.
Supercedes #7381.

* Remove event handler used in testing.

* Fix sending message to the non-existant nation.

* Do the same for TownUtil, removing call to sendPrefixedNationMessage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment