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

raidboss: re-add territory replaced by 6.2 revamp #5116

Merged
merged 3 commits into from
Nov 22, 2022

Conversation

sandtechnology
Copy link
Contributor

fix #5115, old territory id still needed for pre-6.2 game (cn/ko)

fix quisquous#5115, old territory id still needed for pre-6.2 game (cn/ko)
@xiashtra
Copy link
Contributor

Instead of duplicating the trigger files for the old zoneIds, you can just add them to the existing triggers as an array alongside the 6.2 IDs, along with a TODO to remove them once KO/CN catches up to 6.2.

@sandtechnology
Copy link
Contributor Author

sandtechnology commented Nov 22, 2022

Instead of duplicating the trigger files for the old zoneIds, you can just add them to the existing triggers as an array alongside the 6.2 IDs, along with a TODO to remove them once KO/CN catches up to 6.2.

For why I want to duplicate those files is that the new version may have difference compare to old version, and when someone want to update it, it will become a pain for compatible with old version, so instead putting 6.1 and 6.2 zone id into the same trigger files, I think duplicating is the more comfortable solution.

@quisquous
Copy link
Owner

quisquous commented Nov 22, 2022

I think there are already differences. I updated the Sohm Al headmarkers in 5e84ec8 and so probably that needs to be reverted for the 6.1 version. Do you want to undo that in the 6.1 files?

Agreed that if the files are the same we don't need to duplicate, but I suspect that all of these have changes that nobody's updated the files for, so it's probably best to duplicate up front and we'll just remove the duplicates when everything has updated. EDIT: Sorry, that's maybe ambiguously worded. I'm saying I agree that duplicating files is the right approach, since there are likely changes across game versions even if the files aren't updated yet.

@sandtechnology
Copy link
Contributor Author

sandtechnology commented Nov 22, 2022

I think there are already differences. I updated the Sohm Al headmarkers in 5e84ec8 and so probably that needs to be reverted for the 6.1 version. Do you want to undo that in the 6.1 files?

Agreed that if the files are the same we don't need to duplicate, but I suspect that all of these have changes that nobody's updated the files for, so it's probably best to duplicate up front and we'll just remove the duplicates when everything has updated. EDIT: Sorry, that's maybe ambiguously worded. I'm saying I agree that duplicating files is the right approach, since there are likely changes across game versions even if the files aren't updated yet.

Thanks for reminding! I just check those triggers and seems the Sohm AI headmarker is needed to update even after reverted (Due to the 6.0 headmarker changes? I don't know), so I update it and now this PR should be ready for review again.

@quisquous quisquous merged commit 12c8c31 into quisquous:main Nov 22, 2022
@quisquous
Copy link
Owner

Thank you for this!

github-actions bot pushed a commit that referenced this pull request Nov 22, 2022
)

fix #5115, old territory id still needed for pre-6.2 game (cn/ko) 12c8c31
github-actions bot pushed a commit that referenced this pull request Nov 22, 2022
)

fix #5115, old territory id still needed for pre-6.2 game (cn/ko) 12c8c31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

De-synced resources for pre-6.2 game due to the changes in 6.2
3 participants