Don't say selected sectors "show up in reverse video", that depends on
the client. Just say "will be highlighted", like we do for the other
commands that highlight, e.g. info survey.
read can print telegram without header after deletion prompt
rea() loops if more telegrams arrive while we wait for the player to
confirm deletion. If the first new one is a continuation of the last
old one, its header is suppressed. Don't do that.
Fix 'm' in path argument of explore, move, transport
Two related bugs:
* It moans about deprecated argument syntax ('m' without a space
before its argument) even when there's no argument.
* It uses the third instead of second argument for map flags (second
argument is ignored): "m# s" doesn't show ships, and "m# s p" shows
planes instead of ships.
Make market command unavailable to visitors, and set C_MOD flag
The market command executes all trades that have become ready, by
calling check_market() and check_trade(). This modifies game state,
but market lacks C_MOD. That's wrong. But can it do harm?
Turns out yes. check_trade() looks safe, but check_market() telexes
seller and buyer while holding a copy of the commodity struct. If
this telexes the player who sent the market command, and he has
NF_INFORM on, the telegram notification may yield the processor. It
then writes back its copy, triggering a generation oops. Any updates
made by other threads meanwhile are wiped out, triggering a seqno
mismatch oops.
This can cause commodity trades to be executed multiple times,
multiplying the sold commodities. Abuse seems tricky, but possible:
conspiring trade partners trade commodities back and forth to multiply
them. One of them needs to get the output backlog just right to make
the telegram notification yield, and the timing right so that the
MarketUpdate thread or another player's buy, market, reset, sell, set
or trade command runs check_market() before his thread resumes.
Closely related bug: visitors can trigger execution of trades by means
of command market. That's clearly inappropriate.
Broken in Empire 3. Reported by Scott C. Zielinski.
When a ship, plane, land unit or nuke is sold, the seller is replaced
by POGO: POGO gets the money, the telegrams and makes the news.
Likewise when a sale fails because the buyer can't pay.
Broken in commit 94a3108b, v4.3.17. Reported by Scott C. Zielinski.
Fix server shutdown to let player output drain properly
Commit 1e1dfc86 (v4.3.23) attempted to do this, but it's flawed.
Server shutdown makes the player command loops terminate. Each player
thread then flushes buffered output and closes the server's end of the
connection. The client eventually gets EOF, and closes its end of the
connection. All is well.
However, if the client sends more input after the server closed its
end of the connection, but before it completed receiving server
output, it gets ECONNRESET, and the remaining output is lost.
Instead of closing the server's end of the connection, we need to shut
down its transmission direction, then wait for the client to close its
end, by receiving client input until EOF. Do that in io_close().
The output flushing in player_login() is now superfluous. Remove it.
Make shutdwn() wait for the io_close() to complete instead of output
queues to drain. Without that, we could still close the server's end
of the connection prematurely, through program termination. Change
player_delete() to keep the player in Players until after io_close()
completes, so that shutdwn() can detect completion.
quit_cmd() calls io_shutdown() to make player_login()'s next command
read detect EOF. io_shutdown() drains the input queue and shuts down
the socket with shutdown(). player_login()'s next io_gets() fails all
right, but then io_input() *can* read more from the socket on my Linux
box, at least when I send plenty of input fast. Thus, we ignore
whatever input after quit was already queued, then resume reading
commands, not necessarily at the beginning of a line.
Timeout during execute gets handled just like an EOF cookie: end the
batch file, resume reading normal commands. That's wrong, we need to
close the connection.
A real EOF is recorded in the player's connection's EOF indicator.
Let's use that for all "connection needs to be closed" conditions, so
they all work the same. Create io_set_eof() to provide access.
Make recvclient() set the player connection's EOF indicator on
timeout. This makes the timeout "stick". Record receipt of an EOF
cookie in new struct player member got_ctld. Also abort the command,
as before. This leaves further interpretation of the EOF cookie to
the command loops.
Make player_main() set the player connection's EOF indicator on
got_ctld. Player connection gets closed on on EOF cookie, as before.
Change execute() to break the batch command loop when got_ctld is set,
then reset it. Ends the batch file on EOF cookie, as before.
Change status() back to checking EOF and error indicators (partial
revert of commit 9c5854c8, v4.3.16), and drop struct player member
eof.
Make execute yield the processor after every command
A player sending execute script contents quickly could theoretically
get an unfair share of the server. Missed in commit db6fd8da
(v4.2.22), which made only the normal command loop yield.
Don't lose output when client shuts down input transmission
player_login() skips sending C_EXIT and flushing server output when
io_eof() is true. That's the case after a read from the socket
returned zero, which means the client has shut down transmission on
his socket, or closed it. If it's the former, then dropping output
like that is bad. Our client never does that, but others might.
Condition was introduced in Empire 2, don't know why.
status() informs the player of non-trivial command costs and earnings,
and when he goes broke or becomes solvent. However, this is skipped
when the command gets aborted by the player signalling EOF, or by
server shutdown, and after a quit command.
Fix by moving the check for EOF or shutdown down to the may_play_now()
check.
This looks a bit like it would also fix charging of play time. But
that's not broken, because player_main() charges, too.
Fix client's command abort at beginning of first input line
Commit 3cceb59b (v4.3.26) fixed the client to abort commands reliably
on ^C, even when it arrives at the beginning of an input line. Except
it didn't work at the beginning of the first input line, because
input_eol was initialized to zero.
Easily fixed, but "end of line" isn't quite right there. Revert sense
and rename to partial_line_sent.
Simplify journal_entry_pr(), rename to journal_entry_write()
journal_entry_pr(S, N) writes up to N characters from zero-terminated
string S. journal_input() passes -1 for N to write all characters.
Unclean. SIZE_MAX would do, but it's C99, and MSC doesn't provide it.
Simplify journal_entry_pr() to write exactly N characters. This makes
it more similar to write() than to pr(), therefore rename.
drop and fly from carrier can fail to load last civ or mil
pln_equip() refuses to abandon its base sector. Unfortunately, it
checks even when flying off carriers, and refuses to load the last
civilian or military depending on what happens to be in uninitialized
variable sect.
Change read and wire to never delete a corrupt mailbox
Before, rea() deleted the mailbox regardless of errors. Acceptable
only when the user gets a chance to avoid that after the problem is
reported. Not the case for "read y".
Not an issue for announcements, but treat them the same for
simplicity.
Clean up how read deals with new telegram arrivals
Fooling around with the file size is silly. It works only because
read has flag C_MOD set, so they can only arrive while we're sitting
at the delete confirmation prompt, not during reading.
Change wire not to read more after confirming deletion
Telegram deletion deletes the mailbox. If more telegrams arrive while
we wait for the player to confirm deletion, the mailbox again contains
unread telegrams, so we can't just delete it. Instead, rea() loops to
read the new telegrams.
Announcements worked the same until Empire 3 put them in a single file
shared by all. Since then, deleting announcements merely updates
nat_annotim, and there's no need to read new announcements after
getting the player's confirmation. So don't.
Don't lie "You have a new telegram" after read with inform off
Here's how telegram notification works with NF_INFORM off: typed_wu()
increments the telegram recipient's nat_tgms. status(), running right
before command prompts, notifies the player when nat_tgms > 0, and
resets it. Thus, we tell the player how many telegrams arrived since
the previous command prompt.
However, what we really want is something else, namely the number of
"new telegrams waiting". That's what the notification message says,
after all. Telegrams already printed by read shouldn't count, even
when they arrived since the previous command prompt.
Make them not count by clearing pending telegrams on read regardless
of toggle inform.
Fix telegram notifications during read (toggle inform on)
Reset number of pending telegrams before delete prompt instead of
after.
Before, the client claimed pending telegrams at that prompt, because
it wasn't C_INFORMed of the read, yet. Worse, if more telegrams
arrived while sitting at the prompt, the reset clobbered their number
and sent a bogus clear C_INFORM message, effectively hiding the new
arrivals from the player.
Fix pending anno count for annos squashed together
Adjacent announcements are squashed together when the sender is the
same and the timestamp is "close enough". Except typed_wu()
increments natstr member nat_ann regardless. Fix that to work exactly
like nat_tgms.
Fix read not to split production report when update is slow
Adjacent telegrams are squashed together if type and sender are the
same, and the timestamp is "close enough". This is done in two
places: rea() and typed_wu(). They're inconsistent: typed_wu()
ignores the timestamp for production reports since Empire 2, but rea()
doesn't.
Record typed_wu()'s decision in new telstr member tel_cont. Use it in
rea().
typed_wu() counts telegrams to update nat_tgms and, since Empire 2,
send C_INFORM messages. Adjacent telegrams are squashed together if
type and sender are the same, and the timestamp is within TEL_SECONDS.
typed_wu() increments nat_tgms when it sends a telegram that read
doesn't squash into the previous one.
Since Empire 2, it also sends a C_INFORM message then. Inexplicably,
it fails to use the same condition: it tests just new_tele, not
new_tele || np->nat_tgms == 0. C_INFORM messages got missed, until
4.0.18 made rea() call clear_telegram_is_new(). Convoluted.
Send C_INFORM exactly when incrementing nat_tgms, and back out
4.0.18's fix.
Move clear_telegram_is_new() call to beginning of update
The call was added in 4.2.5 "so that the next telegram is flagged as
new and not part of the update". Since the update sends only
TEL_UPDATE telegrams (the previous commit restored that property), and
nothing else does, the next telegram is flagged as new automatically,
except when it's from the next update. Document that, and move the
call to a more natural place.
Ensure the update's production report isn't split by bulletins
During the update, wu() sends TEL_UPDATE telegrams ("Production
Report") instead of TEL_BULLETIN telegrams, but typed_wu() has no such
logic. It's used by tele(), which doesn't run during the update, and
mpr(), which may, e.g. called from ship or mission code used by
autonav or sail. This inserts bulletins in the middle of the
production report, splitting it apart.
Happens since mpr() was added in Empire 2. Before, only tele() used
typed_wu() directly, and everything else wu().
Leave nat_ann, nat_tgm alone and return -1 on all errors. Before,
only failed open was handled that way. Failed write and close were
logged and ignored. While there, improve the log messages a bit.
Note: the return value fix has little effect. It makes tele() log the
failure, which is redundant. Everything else goes through wu() and
ignores the value.
Make add require confirmation for unadvisable actions
Deleting a country in state STAT_SANCT, STAT_ACTIVE or STAT_GOD is
risky, because any references to this country become dangling, which
makes ef_verify() unhappy. For a reason: we may well have code that
isn't prepared for dangling references, and breaks.
Replacing a country that is being used is risky, because it can get us
into weird states. For instance, replacing a player by a visitor can
result in a visitor that owns stuff.
The add command sets nation flags NF_FLASH, NF_BEEP, NF_COASTWATCH,
NF_SONAR, and since 4.2.6 NF_TECHLISTS. POGO is created by the files
utility, which sets only NF_FLASH (since 4.2.2). Change files to
match add.
Before, add reset the country only when adding a player or a visitor.
When adding a deity or deleting a country, it set just nat_cnam,
nat_pnam and nat_state. Has always been that way.
Because of that, a newly minted deity country could inherit all kinds
of crap from a previous user of its country number: origin, realms,
relations, telegrams, ... Harmless if the country number has never
been used before, which is how add is generally used.
When adding a deity country, initial levels (start_education, ...) now
apply, relations start NEUTRAL instead of AT_WAR, and the usual
initial nation flags are set.
Reset on delete as well, just to get rid of the special case.
Argument "active" is obscure. It creates a country in STAT_ACTIVE
that doesn't have a capital, and has its origin at the true origin.
If you really want such a country, create it in STAT_NEW normally,
then use edit to go to STAT_ACTIVE.
0 <= fd < FD_SETSIZE must hold, or else undefined behavior in FD_SET()
and buffer overrun in LwpFdwait[fd]. Check of upper bound off by one,
check of lower bound missing.
sell used to search multiple sectors for sellable commodities, keeping
tally in totalcom. It failed with message "No eligible" when none
could be found.
sell's second argument got changed to a single sector in Empire 3. If
the sector can't sell, we return early. Else, totalcom is positive.
Thus, the "No eligible" code is unreachable. Remove it.
POGO can navigate dead ships, and march dead land units. The ghosts
even get sighted and interdicted, and can hit mines (landmines only
until commit fe372539, v4.3.27). Noted for ships in commit 9100af0b.
Has always been broken. Fix by making shp_sel() and lnd_sel()
explicitly reject ghosts.
Same code pattern also exists in pln_sel, but dead plains fail the
efficiency test, so it's harmless there. Apply the same fix anyway.
Fix "sector can board" test to ignore land units on land units
boar() lets a sector board if it has mobility or usable land units.
Embarked land units are not usable. But it tests only "on ship", not
"on land unit". Broken in 4.0.17.
Fix transport to reject planes loaded on land units
The transported plane remains on its carrier. When the land unit
moves, the plane is teleported right back to it. Broken since
Chainsaw 3 added land units.
While there, improve the message for planes on ships.
Fix prewrite callbacks' cargo list update for in-place update
When updating in-place (old==new), we must not write through new
before we're done reading the same memory through old.
Bug: we write the carrier uids too early. Cargo lists aren't updated
when a carrier dies in an in-place update. No such updates are known.
Broken in commit 64a53c90, v4.3.17.
They didn't since commit 93d033cf, v4.3.26. Drawback: micromanagement
incentive to unload them for the update. Similar incentive has always
existed for military on ships.
Since the previous commit, land units loaded on land units get
unloaded when the carrier dies fighting che. Such land units get
stuck in the sector if the take over, and can be boarded. Doesn't
feel right, and increases the micromanagement incentive. Avoid by
letting them fight.
Don't leave cargo stuck on land unit killed by che
When che destroy a land unit, any embarked units remain stuck on their
now dead carrier. Closely related to and same impact as the bug fixed
in commit 8ccad0d7. Broken since Chainsaw 3 added land units.
The obvious fix would be to match what normally happens when a carrier
gets destroyed: destroy the cargo. Requires recursion. To keep
things as simple as possible, destroy plane cargo, but unload land
unit cargo. That way, the only cargo of cargo to visit are nukes on
planes.
Unloading the land units creates another problem, which will be
addressed in the next commit.