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.

Sign up for GitHub

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

Jump to bottom

Check for invalid combination before calling DefaultAI::expedition_management() #6438

Closed
wants to merge 2 commits into from
Closed

Check for invalid combination before calling DefaultAI::expedition_management() #6438

wants to merge 2 commits into from

Conversation

bunnybot
Copy link

tothxaMirrored from Codeberg
Created on Thu Apr 25 00:20:42 CEST 2024 by Tóth András (tothxa)

Type of change
Bugfix

Issue(s) closed
Fixes #6436

New behavior
DefaultAI::expedition_management() is only called if ship is really an expedition, otherwise the ShipObserver's waiting_for_command_ is cleared.

Possible regressions
none?

@bunnybot bunnybot added this to the v1.3 milestone Apr 24, 2024
@bunnybot bunnybot self-assigned this Apr 24, 2024
@bunnybot
Copy link
Author

Assigned to tothxa

@bunnybot bunnybot added bug Something isn't working crashes or hangs Assert failures, sigsegv, crashes, Widelands hangs, … seafaring Ships, ports, naval invasions, … ai Computer players ci:success CI checks succeeded labels Apr 24, 2024
@bunnybot
Copy link
Author

frankystoneMirrored from Codeberg
On Sat Apr 27 16:02:42 CEST 2024, frankystone wrote:

Since this might be problematic in every game vs an ai player, shouldn't this be a release fix (v1.2-rc2)?

@bunnybot bunnybot mentioned this pull request Apr 27, 2024
Closed
hessenfarmer
hessenfarmer approved these changes Apr 28, 2024
View reviewed changes
Copy link
Contributor

@hessenfarmer hessenfarmer left a comment

Choose a reason for hiding this comment

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

LGTM. Seems to have been tested by frankystone

@bunnybot bunnybot added the review:approve The pull request has been approved. label Apr 28, 2024
@bunnybot
Copy link
Author

bunnybot commented May 1, 2024

hessenfarmerMirrored from Codeberg
On Wed May 01 10:49:22 CEST 2024, Stephan Lutz (hessenfarmer) wrote:

Since this might be problematic in every game vs an ai player, shouldn't this be a release fix (v1.2-rc2)?

+1 from my side. As we got quite some reports after the release reporting crashes and segfaults and other bugs shall we plan for a small bugfix intermediate release before going to the 1.3?
<@>Nordfriese what do you think?

@bunnybot
Copy link
Author

bunnybot commented May 1, 2024

NordfrieseMirrored from Codeberg
On Wed May 01 11:04:02 CEST 2024, Benedikt Straub (Nordfriese) wrote:

+1 from me too.

If we publish a bugfix point release I'd also like to include all the other recent smallish bugfixes as well, namely:

  • c51189f (Skip invalid map files in editor Load/Save map window)
  • b0c4c02 (Fix memory leaks in main menu Watch Last Replay)
  • b166b0b (Game setup: Unselect incompatible starting conditions when changing win condition)
  • 1907f67 (Fix soldier request for expedition ships)
  • 1babc3a (Add thread identifier to crash reports)

Regardless of how many such fixes we backport I'm going to make that a one-week release candidate (1.2.1~rc1) followed by a point release 1.2.1.

@bunnybot bunnybot added ci:fail CI checks failed and removed ci:success CI checks succeeded labels May 1, 2024
@tothxa tothxa removed the ci:fail CI checks failed label May 1, 2024
@bunnybot bunnybot added the ci:fail CI checks failed label May 1, 2024
@bunnybot
Copy link
Author

bunnybot commented May 1, 2024

tothxaMirrored from Codeberg
On Wed May 01 13:36:07 CEST 2024, Tóth András (tothxa) wrote:

The CI failure was transient, re-running the failed jobs succeeded, but apparently bunnybot can't handle that.

@bunnybot
Copy link
Author

bunnybot commented May 1, 2024

NordfrieseMirrored from Codeberg
On Wed May 01 13:53:14 CEST 2024, Benedikt Straub (Nordfriese) wrote:

<@>bunnybot merge force

@bunnybot bunnybot closed this May 1, 2024
@bunnybot bunnybot deleted the mirror/tothxa/widelands/ai-expedition-check branch May 1, 2024 11:56
bunnybot pushed a commit that referenced this pull request May 1, 2024
@tothxa
Check for invalid combination before calling DefaultAI::expedition_ma…
145b2ab 
…nagement() (CB #4800 / GH #6438)

Co-authored-by: Tóth András <txa-dev@posteo.hu>
Co-committed-by: Tóth András <txa-dev@posteo.hu>
Noordfrees pushed a commit that referenced this pull request May 1, 2024
@tothxa @Noordfrees
Check for invalid combination before calling DefaultAI::expedition_ma…
cf3d48a 
…nagement() (CB #4800 / GH #6438)

Co-authored-by: Tóth András <txa-dev@posteo.hu>
Co-committed-by: Tóth András <txa-dev@posteo.hu>
@hessenfarmer
Copy link
Contributor

NordfrieseMirrored from Codeberg On Wed May 01 11:04:02 CEST 2024, Benedikt Straub (Nordfriese) wrote:

+1 from me too.

If we publish a bugfix point release I'd also like to include all the other recent smallish bugfixes as well, namely:

* [c51189f](https://github.com/widelands/widelands/commit/c51189f67aa06b2273486134473f17c7a014ace8) (Skip invalid map files in editor Load/Save map window)

* [b0c4c02](https://github.com/widelands/widelands/commit/b0c4c025befe3c3c09226b52cdd5d41446ffe16b) (Fix memory leaks in main menu Watch Last Replay)

* [b166b0b](https://github.com/widelands/widelands/commit/b166b0b52a7da5dc88c74b079149cc46e806de81) (Game setup: Unselect incompatible starting conditions when changing win condition)

* [1907f67](https://github.com/widelands/widelands/commit/1907f67c6978b4dc8717e0c84670727940eea80b) (Fix soldier request for expedition ships)

* [1babc3a](https://github.com/widelands/widelands/commit/1babc3aac05757ac25c6992da13afa6c41a6cec5) (Add thread identifier to crash reports)

Regardless of how many such fixes we backport I'm going to make that a one-week release candidate (1.2.1~rc1) followed by a point release 1.2.1.

including all recent fixes of crashes was indeed what I had in mind, while asking. But we should go over the list of recent bug reports and see what would be worth fixing as well. i.e. bugs that are deemed critical or really annoying.
Furthermore I`d like to add the criterion that we should have a special look to bugs reported by users other then the dev team to show that it is worth it to sen bug reports. We might even thank the community for finding and especially reporting them.

@bunnybot bunnybot modified the milestones: v1.3, v1.2.1 May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hessenfarmer hessenfarmer hessenfarmer approved these changes

Assignees

@bunnybot bunnybot

Labels
ai Computer players bug Something isn't working ci:fail CI checks failed crashes or hangs Assert failures, sigsegv, crashes, Widelands hangs, … review:approve The pull request has been approved. seafaring Ships, ports, naval invasions, …
Projects
None yet
Milestone
v1.2.1
Development

Successfully merging this pull request may close these issues.

Crash while playing
3 participants
@bunnybot @hessenfarmer @tothxa