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

Eastern Invasion: add Dalas's version to mainline #7815

Merged
merged 29 commits into from
Sep 15, 2023
Merged

Eastern Invasion: add Dalas's version to mainline #7815

merged 29 commits into from
Sep 15, 2023

Conversation

nemaara
Copy link
Contributor

@nemaara nemaara commented Jul 26, 2023

Hi all, I've taken Dalas's EI revised from the addon server and prepped it for mainline. We've gone through one round of review and are on the second now. The review items in order of importance are:

  • Playtesting and bug reports
  • Code review
  • Typo review
  • Balancing
  • Anyone wanting to clear some of the TODOs in the scenarios is welcome to do so too.

I'm going to aim to get this in master by September 15. This time, let's please not do a line by line dialogue review and instead keep it to general comments. Thanks!

@github-actions github-actions bot added AI Issues with the AI engine, including micro AIs. Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign Graphics Issues that involve the graphics engine or assets. Audio Issues with the sound engine. Units Issues that involve unit definitions or their implementation in the engine. Terrain Issues that involve terrain definitions or their implementation in the engine. Achievements Issues with the Achievements Functionality labels Jul 26, 2023
@nemaara nemaara self-assigned this Jul 26, 2023
@soliton-
Copy link
Member

soliton- commented Jul 26, 2023

Instead of the *-credits.txt files this info should be added to a general ART_LICENSE. (And eventually we should ideally add all the existing stuff, too. There is already data/core/images/portraits/ARTISTS specifically for portraits.)

@Jonathan-Kelly
Copy link
Contributor

Regarding these changes to S03_An_Unexpected_Appearance... any chance of keeping the original versions of these two nasty goth siblings here? I got a kick out of them when I played the add on version. I find the humour (both in S3 and the brick joke in S16) works less well with one of them changed to a sympathetic character.

cooljeanius added a commit to cooljeanius/A_New_Order that referenced this pull request Aug 25, 2023
Copy link
Contributor

@gfgtdf gfgtdf left a comment

Choose a reason for hiding this comment

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

I lost a bit track here tbh, but i think i don't see anything that needs to be addressed before merging. (commenting because my review was requested)

Co-authored-by: Luis Miguel Iglesias Sánchez <Luis_koopa@hotmail.com>
@cooljeanius cooljeanius mentioned this pull request Sep 6, 2023
31 tasks
@nemaara
Copy link
Contributor Author

nemaara commented Sep 9, 2023

One week call on the PR for feedback, will merge next Friday ish if nothing else is pressing.

@nemaara
Copy link
Contributor Author

nemaara commented Sep 14, 2023

One day call! Last chance for comments before merging.

@nemaara nemaara merged commit 6c03122 into wesnoth:master Sep 15, 2023
@nemaara nemaara deleted the EI-rev branch September 15, 2023 17:44
@Dalas121
Copy link
Contributor

Woot! Thanks to everyone for all your feedback and work getting this ready for mainline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Achievements Issues with the Achievements Functionality AI Issues with the AI engine, including micro AIs. Audio Issues with the sound engine. Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign Graphics Issues that involve the graphics engine or assets. Terrain Issues that involve terrain definitions or their implementation in the engine. Units Issues that involve unit definitions or their implementation in the engine.
Projects
None yet
Development

Successfully merging this pull request may close these issues.