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

CTD when accepting King's proposal to help with war (Royal Frigate + troops) #854

Closed
nci-wtp opened this issue May 12, 2023 · 9 comments
Closed
Assignees

Comments

@nci-wtp
Copy link

nci-wtp commented May 12, 2023

This happened in a number of my games. The issue is 100% reproducible with the save file attached.

AutoSave_AD-1517-January.ColonizationSave.txt

@Nightinggale
Copy link
Contributor

CvPlayer::handleDiploEvent()
case DIPLOEVENT_ROYAL_INTERVENTION:
CvCity *locationToAppear = kPlayer.buyUnitFromParentPlayer(getID(), "UNITCLASS_ROYAL_INTERVENTIONS_SHIP", 1, "", 0, LocationFlags::LocationFlagDeepCoastal, false, false);

locationToAppear is NULL, yet it is used without checking for NULL, specifically it tries to print the name of the city.

Same savegame has an assert failure in CvPlayer::changeUnitClassMaking where the number for bishops is changed to -1.

Right now I have more questions than answers, but at least this bug can be easily reproduced and as such testing is less of an issue than it has been with some other recent bug reports.

@Nightinggale Nightinggale self-assigned this May 16, 2023
@nci-wtp
Copy link
Author

nci-wtp commented May 16, 2023

At least on some occasions with this bug, I was offered King's help for less than it's nominal value of 8550 gold. Maybe this information can be helpful...

@Nightinggale
Copy link
Contributor

Fixed the crash. The new ship is now placed in Europe rather than crashing on a NULL city.

I found one more bug in this savegame. First the king asks you to make peace with the French and regardless of your answer, then next thing the king says is the war is dragging on and he offers to sell units. This means if you accept peace, he will offer units for the war, which isn't there. This in turn seems to make the DLL set the enemy player to player 0 (yourself), through I'm not sure that hurts anything other than possibly log entries.

In short while the originally reported bug has been fixed, the savegame is still valid for bug hunting.

@nci-wtp
Copy link
Author

nci-wtp commented Jun 25, 2023

@Nightinggale in another game, I received only a ship, and no troops, when ship was placed in Europe.

The issue is reproducible with the save file in first post.

@raystuttgart
Copy link
Collaborator

raystuttgart commented Jun 25, 2023

Are you really talking about WTP 4.1 (develop)?

Because the "Royal Interventions" should NOT even trigger in case there is no "deep water coast".
There is no such logic that it should in that case try to place any Ship in Europe ...

The only thing that may try to place Ships in Europe are the Python Events.
(No DLL Diplo Event should do so.)

@raystuttgart
Copy link
Collaborator

raystuttgart commented Jun 25, 2023

Edit:
I see that somebody rewrote my logic in some way.
So I just do not know what happens anymore.

This code was definitely not written by me ...
But I also see no comments so I do not know who wrote it.

				LocationFlags location;
				location.deepCoastal = true;
				location.europe = true;

Generally the logic for these DLL Diplo Dialogues should simply never trigger (as there are already safety checks for that) in case the Player has no Deep Harbor Coast.

@raystuttgart
Copy link
Collaborator

raystuttgart commented Jun 25, 2023

Also I am really not sure why it spawns Royal Ships and Soliders in Europe then instead of simply on the Starting Plot.
(Which would have been the much much easier and less confusing solution.)

Ah ok, I see.
@Nightinggale implemented the spawning in Europe on purpose.

@nci-wtp
Copy link
Author

nci-wtp commented Jun 25, 2023

@raystuttgart

Are you really talking about WTP 4.1 (develop)?

Of course, updated couple hours ago.

Generally the logic for these DLL Diplo Dialogues should simply never trigger (as there are already safety checks for that) in case the Player has no Deep Harbor Coast.

That would be unreasonably punishing for players without cities with (deep) coast tiles, don't you think?

@raystuttgart
Copy link
Collaborator

raystuttgart commented Jun 25, 2023

I was just explaining what my old logic did or at least was supposed to do - not trying to justify if it was wrong or right.
But right now it behaves completely different than what I had implemented / known ... thus I am surprised and cannot really help.

Comment:
Spawning the Units on Starting Plot may still be a lot easier and safer than in Europe.
But ok, Europe is also fine for me.

Nightinggale added a commit that referenced this issue Oct 16, 2023
… if the ship is added in Europe

When creating the units, first the ship is added in a deep water port with Europe as fallback.
The units were not included in the previous fix and were still stuck with deep water port only.
Now they too have Europe as fallback.
Nightinggale added a commit that referenced this issue Oct 17, 2023
…l back to Europe if none are found

Also fixed diplo events spawning land units in Europe. They now actually appear on the dock.
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

No branches or pull requests

3 participants