Skip to content

Conversation

@ATFGK
Copy link
Contributor

@ATFGK ATFGK commented Nov 28, 2022

Fixes #2804

Fix GetDataHandlers.cs file for version 144X, and correct various errors caused by it.

@drunderscore
Copy link
Contributor

As originally requested in #2823, please don't include changes to number of PVE/PVP deaths, as that will be dealt with in #2768

Commits and changes are still a bit scattered, and it's not super clear which issues this PR solves -- can you update the description to include the specific issues that this PR solves?

@ATFGK
Copy link
Contributor Author

ATFGK commented Nov 28, 2022

As originally requested in #2823, please don't include changes to number of PVE/PVP deaths, as that will be dealt with in #2768

Commits and changes are still a bit scattered, and it's not super clear which issues this PR solves -- can you update the description to include the specific issues that this PR solves?

1.Fixed wrong MaxProjDamage trigger.
2.Fix the wrong resurrection prompt.
Once I delete the content related to PVE/PVP, the second problem will not be solved.

@drunderscore
Copy link
Contributor

@ATFGK I'm not sure I understand that last issue -- can you describe it further or show an example?

@ATFGK
Copy link
Contributor Author

ATFGK commented Nov 28, 2022

@ATFGK I'm not sure I understand that last issue -- can you describe it further or show an example?

It has little impact on "Tshock", but the wrong "SpawnContext" message will cause a lot of problems for other plug-ins because they cannot correctly recognize "SpawnContext"

@ATFGK
Copy link
Contributor Author

ATFGK commented Nov 28, 2022

@ATFGK I'm not sure I understand that last issue -- can you describe it further or show an example?

It has little impact on "Tshock", but the wrong "SpawnContext" message will cause a lot of problems for other plug-ins because they cannot correctly recognize "SpawnContext"

"Tshock" also uses a lot of "TSPlayer. Spawn()", so other plug-ins cannot avoid receiving incorrect "SpawnContext"

@drunderscore
Copy link
Contributor

Yeah, totally see the issue and understand. With that other PR blocked for non-working, I'm fine accepting these changes first, even if it means the other PR will get conflicts.

I'll review this ASAP.

Copy link
Contributor

@drunderscore drunderscore left a comment

Choose a reason for hiding this comment

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

All the code looks good -- just some code smell and grammatical issues otherwise!

@ATFGK ATFGK requested a review from drunderscore November 30, 2022 00:55
ATFGK and others added 2 commits November 30, 2022 09:01
Missing letters due to input method problems.
Copy link
Contributor

@drunderscore drunderscore left a comment

Choose a reason for hiding this comment

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

Unfortunately the changelog quality is still quite lacking -- so instead of holding this PR up for any longer due to it's non-conforming changelog, please copy the changelog entries from this changelog file into yours.

@drunderscore
Copy link
Contributor

To make it even easier, I've created a pull request on your repository for this branch with the correct changes.

Correct changelog entries for Pryaxis/TShock Pryaxis#2852
@ATFGK
Copy link
Contributor Author

ATFGK commented Dec 1, 2022

To make it even easier, I've created a pull request on your repository for this branch with the correct changes.

Thank you very much for your help. :-D

@ATFGK ATFGK requested a review from drunderscore December 3, 2022 07:04
@hakusaro hakusaro merged commit 95d157f into Pryaxis:general-devel Dec 6, 2022
@ATFGK ATFGK deleted the Update-GetDataHandlers.cs branch December 6, 2022 06:50
THEXN pushed a commit to THEXN/TShock that referenced this pull request Feb 22, 2025
)

* update

* update

* update

* Update changelog.md

* Update changelog.md

Fixed syntax errors.

* Update SpawnMsg.cs

Use insteadUpperCamelCase

* Update changelog.md

* Update TSPlayer.cs

* Update TSPlayer.cs

Missing letters due to input method problems.

* Update `docs/changelog.md`

Co-authored-by: James Puleo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terra Blade triggered MaxProjDamage Etc

3 participants