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

Fix peacock escalations' incomplete displayed killing tips #358

Merged
merged 64 commits into from
Jan 10, 2024

Conversation

suanjiansalt
Copy link
Contributor

@suanjiansalt suanjiansalt commented Dec 23, 2023

Improvement program for issue #353 , huge thanks to @grappigegovert .

A dumb try, so please treat this as a pupil's homework, after all it's my first time doing this...

Note:

  • Here I also did some adjustments to The Christmas Calamity gameplay. If it's broken, I'm appreciated to receive your kind advice. This time I fixed it correctly. If it's unnecessary, I will rollback it in no time.
  • Icon shown by HUDTemplate line looks improper in The Tedious Thievery. Before you collect all items (7 golds and 2 burgers) the icon is exactly the 'Exit' icon, but after completion the icon is what we want (a blue 'objective' icon). I deleted OnActive line but this could be ridiculous. Please inform me before I look too stupid. I changed the iconType to 16 and it looks good now.
  • Maybe all targets' name localization is required to realize?

Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Maybe it's a way to fix icon error in tedious thievery? (icons for counting stolen golds and burgers are exit icons)

Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
@suanjiansalt suanjiansalt marked this pull request as ready for review December 23, 2023 13:53
@@ -76,7 +80,12 @@
},
{
"$inarray": {
"in": "$.LethalWeapons",
"in": {
"$or": [
Copy link
Member

Choose a reason for hiding this comment

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

Does this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, but maybe I used too much imagination and creativity here. You may see that all I want to do is to take Christmas explosives into consideration, so maybe there's a more acceptable type exist?

Copy link
Member

Choose a reason for hiding this comment

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

You could just add the two repo ids of the explosives to the LethalWeapons array. I'm not entirely sure if explosive weapons can be checked like this however, so I suggest testing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense... So the final question is, how to test? 🤯

Copy link
Member

Choose a reason for hiding this comment

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

Play the contract and check if killing a target with your added explosives changes the counter?

Copy link
Contributor Author

@suanjiansalt suanjiansalt Dec 23, 2023

Choose a reason for hiding this comment

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

I mean which folder to place this JSON in

Copy link
Member

Choose a reason for hiding this comment

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

You can either follow the steps in the Readme.md under "Installation", and just run a dev version of Peacock with your modified files, or if you want you could copy the contract json into the "contracts" folder of your release version of Peacock, and then access the contract through "My contracts" in game.

Copy link
Contributor

@AnthonyFuller AnthonyFuller left a comment

Choose a reason for hiding this comment

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

RSLGTM for l10n. A note: some lines that were previously translated to Chinese are no longer translated.

Chinese version.

Signed-off-by: suanjiansalt <[email protected]>
@suanjiansalt suanjiansalt marked this pull request as draft December 24, 2023 15:14
@suanjiansalt
Copy link
Contributor Author

Decide to localize all killing tip strings. Sorry for the wavering.

Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
Signed-off-by: suanjiansalt <[email protected]>
@suanjiansalt suanjiansalt marked this pull request as ready for review December 25, 2023 13:25
suanjiansalt and others added 2 commits December 26, 2023 19:53
Copy link
Contributor

@AnthonyFuller AnthonyFuller left a comment

Choose a reason for hiding this comment

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

I assume it all works and has been verified.

@grappigegovert
Copy link
Member

grappigegovert commented Jan 1, 2024

All of these "Eliminate <name>" lines without any condition don't really need dedicated localization and create unnecessary work for the translators, the generic UI_CONTRACT_GENERAL_OBJ_KILL should work fine

I've only really looked at the the first contract in the list, MENDIE1, and that one already was wrong: your new edit says that the kill needs to be an explosion accident, where any kill method is sufficient.

It also looks like you changed the locr key UI_PEACOCK_SOUTHERN_COMFORT_OBJ1 to UI_PEACOCK_SOUTHERN_COMFORT_XMASGIFT, but in doing so changed the text for all languages to the english translation.

I'll do some more testing later

@suanjiansalt
Copy link
Contributor Author

All of these "Eliminate " lines without any condition don't really need dedicated localization and create unnecessary work for the translators, the generic UI_CONTRACT_GENERAL_OBJ_KILL should work fine

  1. I did this because in H3, almost official escalations really localized all strings like "Eliminate ". For example, in the Jinzhen Incident:
    Screenshot 2024-01-02 110035
    Just eliminating Shihong Luo and Hui Hou in any conditions are both localized. Again, as I said earlier, if it's not necessary, just delete it.

I've only really looked at the the first contract in the list, MENDIE1, and that one already was wrong: your new edit says that the kill needs to be an explosion accident, where any kill method is sufficient.

  1. Oh YES, it is my fault. Not understand why I did it, but it clearly a fault I made on purpose.

It also looks like you changed the locr key UI_PEACOCK_SOUTHERN_COMFORT_OBJ1 to UI_PEACOCK_SOUTHERN_COMFORT_XMASGIFT, but in doing so changed the text for all languages to the english translation.

  1. Well, I'm not merely replace the whole line with new key UI_PEACOCK_SOUTHERN_COMFORT_XMASGIFT. All I did in fact is change the keys' names, so I also give the right value to the keys if you look into it. This happens to some complications of The Jeffery Confession too.

Signed-off-by: suanjiansalt <[email protected]>
@grappigegovert
Copy link
Member

Alright, did a quick test with an xmax explosive, seems to work nicely.

The tileimage for the christmas calamity is broken with this change however, it would need to be updated in the image pack.

Copy link
Member

@grappigegovert grappigegovert left a comment

Choose a reason for hiding this comment

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

Looks all good to me :)

@RDIL RDIL merged commit 97161e8 into thepeacockproject:v7 Jan 10, 2024
5 checks passed
@RDIL
Copy link
Member

RDIL commented Jan 10, 2024

Thanks @suanjiansalt!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants