Most places use "FOO of X,Y changed", some use "FOO in X,Y changed",
"FOO in X,Y was changed", "FOO of sector X,Y changed". Normalize to
the former.
Most places do not end the message with a period, some do. Normalize
to no period.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Key 'L' copies the source sector to a destination sector. Bug: it
doesn't copy, it messes up the source sector badly instead, and can
smash the stack on some machines.
Root cause: doland() passes § instead of sect to ef_set_uid().
Impact:
1. ef_setuid() clobbers a few bytes at §.
When the bitfield and uid fit into sizeof(sect) bytes, it clobbers
just sect, which has no effect, because doland() returns without
using it again. This is the case on a typical 64-bit machine: bit
field and uid are both 4 bytes, sizeof(sect) is 8.
When they don't fit, whatever is adjacent to sect gets clobbered.
On a typical 32-bit machine with stack growing down, that's p.
Again, no effect, because doland() returns without using it again.
With stack growing up, it could well be the return address,
crashing the server.
2. ef_setuid() fails to update *sect. Impact (when we survive 1):
sect->sct_uid remains unchanged. putsect() writes to the source
sector instead of the destination sector, clobbering the source's
sct_x, sct_y. Breaks invariant sctoff(sct_x, sct_y) == sct_uid!
Subsequent edits are all applied to the source sector.
sect->sct_seqno remains unchanged. No effect, because we write to
the source sector, and the unchanged sequence number is the right
one there.
Broken in commit 536ef0b0, v4.3.15.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
edit, setresource and setsector report change in three ways:
* Print a message.
* Send a bulletin to the changed object's owner. This should be done
if and only if the change is visible to the owner, e.g. in census or
xdump.
* Report divine aid in news. This should be done if and only if a
bulletin was sent, except for changes that are neither negative nor
positive, such as changing the distribution sector.
Fix the places that don't get it right for sectors:
cmd key sctstr member before after notes
-------------------------------------------------
edit l O sct_oldown -- B-
edit l F sct_fallout -- BN 1
edit l M sct_mines -- BN 2 3
edit l D sct_dist_x,y -- B- 1
edit l s sct_type -- B- 1
edit l S sct_newtype -- B- 1
setse ow sct_own B- BN
setse ol sct_oldown -- B-
setse e sct_effic -- BN 2
setse mo sct_mobil -- BN 2
setse a sct_avail -- BN 2
setse w sct_work -- BN 2
The two characters in columns before, after show whether the command
sends a bulletin (B) or not (-), and whether it reports news (N) or
not (-).
Notes:
1. Printed message massaged slightly for consistency with other keys.
2. Printed message improved to show the old value, too. Necessary to
give the deity a chance to catch unexpected changes, e.g. a player
laying mines right before the deity edits them. Watching out for
such changes is especially important with non-interactive edit.
3. Bulletin and news suppressed for occupied sectors.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Send bulletin to owner and report news exactly like for key 'o'.
Also print a "sector duplicated" message, just for consistency with
other keys.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Back in Empire 1, resources were limited consistently: fertility to
120, all others to 100.
When Chainsaw added setresource, consistency was lost: fertility got
limited to 100 there.
Chainsaw 3 changed edit to limit all resources to 127.
Commit 3fcee8dd and commit 8e430ae2 (both v4.3.11) changed fairland
and setsector to limit fertility to 100, matching setresource.
Now only edit remains different. Change it to finally make things
consistent again.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When the deity sets the number of mines with setsector, the sector
owner (if any) is told the resulting number of mines. Even for
occupied sectors, where mines belong to the old owner, and thus
shouldn't be disclosed. Oops.
Fix setsector not to tell the sector owner anything then.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Option GODNEWS is documented to be about deities giving or taking
things from players. Nevertheless, edit, give, setsector and
setresource report news of deities meddling with things owned by
deities other than POGO. Don't.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
give already suppresses when the new value equals the old one, but
edit, setresource and setsector don't. Make them.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
In the beginning, all bulletins came from POGO. Chainsaw changed edit
and give to send them from the deity using the command. Its new
command setresource sent from POGO regardless. Its new command
setsector did both.
Go back to sending them only from POGO.
Some of the affected bulletins don't mention the acting deity. Reword
them so they do.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Empty key arguments work fine when passed as command arguments, but
not interactively. For example, 'edit s 42 R ""' clears the retreat
path, but in an interactive 'edit s 43', 'R ""' sets it to "".
In Empire 1, omitting the argument made it empty. Empire 2 turned
that into an error without providing an alternative.
Switch to the common command parser, so that quoting works, and "" is
parsed as empty argument.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Necessary to give the deity a chance to catch unexpected changes,
e.g. a player moving away stuff right before a give command, leaving
fewer items than the deity intends to take.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
give() silently caps the resulting number of items to 0..ITEM_MAX.
However, its test for "< 0" suffers integer overflow on two's
complement machines (i.e. practically everywhere) when the amount
argument is INT_MIN. give() proceeds as if the result was in range:
it sets the number of items to (short)(n + INT_MIN), telexes the owner
that INT_MIN items were stolen (obviously bogus), and tells the deity
that there are now n + INT_MIN items in X,Y.
On common machines, (short)(n + INT_MIN) == n, i.e. nothing is given.
On an oddball machine with short as wide as int, the cast to short
does nothing, item_prewrite() oopses, and corrects the number of items
to zero.
In both cases, output and telegram lie.
Likewise, its test for "> ITEM_MAX" suffers integer overflow for
sufficiently big amount arguments. Again, give() proceeds as if the
result was in range: it sets the number of items to (short)(n + amt),
telexes the owner that -amt items were stolen (obviously bogus), and
tells the deity that there are now close to INT_MIN items in X,Y.
On common machines, (short)(n + amt) = n + INT_MAX - amt - 1,
i.e. some items are stolen.
On an oddball machine with short as wide as int, the cast to short
does nothing, item_prewrite() oopses, and corrects the number of items
to zero.
Again, output and telegram lie.
Aside: setsector can suffer similar overflows, but it reports the
resulting change correctly. Good enough.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Only one of struct lndstr members lnd_ship, lnd_land may be
non-negative. When a deity screws that up, the server oopses. Be
nice: when setting one, zap the other.
Same for struct plnstr members pln_ship, pln_land.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Upper bounds corrected:
cmd key struct member wrong correct notes
-------------------------------------------------
edit l m sct_mobil 255 127 1
t sct_ptime 255 32767
edit c b nat_btu 1024 max_btus
edit p m pln_mobil 255 127 1
edit u F lnd_harden 255 127 1
Missing bounds supplied, arguments out of bounds are silently clipped
unless noted otherwise:
cmd key struct member bounds notes
---------------------------------------------------
edit c t nat_tgms 0 USHRT_MAX 2
m nat_reserve 0 INT_MAX
T... nat_level[] 0 infinity
edit s a shp_pstage 0 PLG_EXPOSED 3
b shp_ptime 0 32767
M shp_mobil -127 127
c... shp_item[] 0 load limit 4
edit u Y lnd_land -1 size of table 5
M lnd_mobil -127 127
S lnd_ship -1 size of table 5
Z lnd_retreat 0 100
c... lnd_item[] 0 load limit 4
edit p r pln_range 0 max range
s pln_ship -1 size of table 5
y pln_land -1 size of table 5
Notes:
1. Values between SCHAR_MAX and 255 were cast to signed char, changing
the sign.
2. The real upper bound is the number of telegrams in the mailbox, but
counting them isn't worth it.
3. This check is particularly important, because values out of bounds
make the server refuse to start without -F, and empdump -x warn
"export has errors, not importable as is".
4. Values outside 0..ITEM_MAX got caught and clipped by
item_prewrite(). This check avoids the oops, and tightens the upper
bound for units.
5. Argument out-of-bounds are rejected. This check is particularly
important, because unit numbers beyond the size of the table trigger
oopses.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Negative numbers and numbers greater or equal than MAXNOC are invalid.
edit handles such arguments inconsistently:
cmd key struct member arg < 0 arg >= MAXNOC
---------------------------------------------------
edit l o sct_own reject MAXNOC - 1
O sct_oldown reject MAXNOC - 1
X sct_che_target 0 MAXNOC - 1
edit s O shp_own cast skip
edit u O lnd_own cast skip
edit p O pln_own cast skip
Legend:
0 replace arg by 0
MAXNOC - 1 replace arg by MAXNOC - 1
reject command fails
cast replace arg by (natid)arg
bug: can be >= MAXNOC!
skip ignore this key and arg
bug: telexes the owner he lost the unit, which is a lie
Unify to reject. Matches setsector.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
This is a fairly comprehensive test of the deity commands to edit game
state: edit, setresource, setsector, give, swapsector.
The test makes edit screw up game state, triggering oopses. The
server refuses to start without -F then, and empdump -x warns "export
has errors, not importable as is". Until these bugs are fixed, skip
this test in "make check".
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Capturing the client's output tests both client and server, which is
nice. However, player input isn't visible in the resulting file,
which makes it more difficult to understand.
Route player output to journal (econfig key "keep_journal 2"), and
ignore client output.
Separate tests for the client would be useful.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Coordinates run into army when the y coordinate is wider than three
characters. Has always been broken. Insert a separating space.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
So you don't have to micromanage workers to maximize useful work.
The previous commit made the problem a bit worse. If you had a few
workers too many before, you perhaps produced an extra unit. Now, you
get to keep the extra work instead. Useless, unless it rolls over.
produce() limits production to how many units the workers can produce,
rounding randomly. It charges work for the units actually produced,
discarding fractions.
If you get lucky with the random rounding, you may get a bit of extra
work done for free. Else, you get to keep the unused work, and may
even be undercharged a tiny bit of work. Has always been that way.
The production command assumes the random rounding rounds up if and
only if the probability to do so is at least 50%. Thus, it's
frequently off by one for sectors producing at their worker limit.
The budget command runs the update code, and is therefore also off by
one, only differently.
Rather annoying for tech and research centers, where a single unit
matters. A tech center with full civilian population can produce 37.5
units in 60 etus. Given enough materials, it'll fluctuate between 37
and 38. Production consistently predicts 38, and budget randomly
predicts either 37 or 38. Both are off by one half the time.
Fix this as follows: limit production to the amount the workers can
produce (no rounding). Work becomes a hard limit, not subject to
random fluctuations. Randomly round the work charged for actual
production. On average, this charges exactly the work that's used.
More importantly, production and budget now predict how much gets
produced more accurately. They're still not exact, as the amount of
work available for production remains slightly random.
This also "fixes" the smoke test on a i686 Debian 6 box for me. The
root problem is that floating-point subexpressions may either be
computed in double precision or extended precision. Different
machines (or different compilers, or even different compiler flags)
may use different precision, and get different results.
Example: producing 108 units at one work per unit, sector p.e. 0.4
needs to charge 108 / 0.4 work. Computed in double precision, this
gets rounded to 270.0, then truncated to 270. In 80 bit extended
precision, it gets rounded to 269.999999999, then truncated to 269.
With random rounding instead of truncation, the probability for a
different result is vanishingly small. However, this commit
introduces truncation in another place. It just happens not to mess
up the smoke test there. I doubt this is the last time this kind of
problem upsets the smoke test.
Broken in commit 3a7d7fa, which enlarged struct natstr member
nat_hostaddr[] from 32 to 46 characters, but neglected to update the
ca_len in nat_ca[]. Consequently, the address is truncated in xdump.
Can also break country * ?ip=... and such, but that's exotic.
emp_server and empdump refuse to start on most big endian hosts,
because ef_verify_config() chokes on mdchr_ca[]:
Config meta uid 0 field type: value 0 is not in symbol table meta-type
Config meta uid 1 field type: value 0 is not in symbol table meta-type
Config meta uid 2 field type: value 0 is not in symbol table meta-type
Config meta uid 3 field type: value 0 is not in symbol table meta-type
Config meta uid 4 field type: value 0 is not in symbol table meta-type
Broken in commit 06a0036 (v4.3.12), which changed struct castr member
ca_type from packed_nsc_type (typedef'ed to char) to enum nsc_type,
but neglected to update the ca_type in mdchr_ca[].
On little endian hosts, the selector reads the least significant byte,
with sign extension. Happens to work, because the type values are all
sufficiently small integers.
On big endian hosts, the selector reads the most signiciant byte.
which is always zero (NSC_NOTYPE). Makes ef_verify_config() fail.
Except when sizeof(enum nsc_notype) == 1. Then selector type works
fine, and ef_verify_config() succeeds, but we run into the next
problem: the same commit also changed member ca_flags from nsc_flags
(typedef'ed to unsigned char) to int without updating the ca_type in
mdchr_ca[]. This breaks "only" xdump meta column flags.
v4.3.12 was released in April 2008. Either nobody has tried to run a
game on a big endian host since, or all who did gave up quietly,
without reporting the problem.
We clearly need to test on a wider range of machines.
Broken in commit 14ea670 (v4.3.8), which changed struct trdstr member
trd_type from char to short, but neglected to update the ca_type in
trade_ca[].
On little endian hosts, the selector reads the least significant byte,
with sign extension. Happens to work, because the type values are all
sufficiently small integers.
On big endian hosts, the selector reads the most signiciant byte,
which is always zero (EF_SECTOR). Messes up xdump trade badly.
Broken in commit 09248d0 (v4.3.8), which changed struct loststr member
lost_type from char to short, but neglected to update the ca_type in
lost_ca[].
On little endian hosts, the selector reads the least significant byte,
with sign extension. Happens to work, because the type values are all
sufficiently small integers.
On big endian hosts, the selector reads the most signiciant byte,
which is always zero (EF_SECTOR). Messes up xdump lost badly. Also
breaks lost * ?type=..., but that's exotic.
Island size is randomly chosen from the interval [1..2*is+1], with
expected value is. Use two dice to roll the size instead of one.
This makes extreme sizes much less likely.
The smoke test waits for the server completing startup by trying to
connect until it works. Hangs if the server doesn't complete startup
for some reason. Make it give up after 5s.
Likewise, The smoke test waits for the server to terminate by trying
kill -0 until it fails. Hangs if the server doesn't terminate. Make
it give up after 5s.
Code dealing with money mixes int and long pretty haphazardly.
Harmless, because practical amounts of money fit into int on any
machine capable of running the server. Clean up anyway.
Code dealing with reserves mixes int and long pretty haphazardly.
Harmless, because practical reserves fit easily on any machine capable
of running the server. Clean up anyway.
As long as symbol_by_value(), show_capab() and togg() support only
int, flags need to fit into int.
Not a problem in practice, because no machine capable of running
Empire has int narrower than 32 bits, and 32 bits suffice.
Some flags members are long instead of int: struct lchrstr member
l_flags, struct natstr member nat_flags, struct mchrstr member m_flags
are long. Waste of space on machines with long wider than int.
Change them to int.
Rearrange struct lchrstr and struct natstr to avoid holes.
random() may yield different pseudo-random number sequences for the
same seed on another system. For instance, at least some versions of
MinGW provide a random() in -liberty that differs from traditional BSD
(see commit c8231b12). Rather inconvenient for regression testing.
MT19937 Mersenne Twister is a proven, high-quality PRNG. Actual code
is reference code provided by the inventors[*]. Quick tests show
performance comparable to random().
Like random(), MT is not cryptographically secure: observing enough of
its output permits guessing its state, and thus its future output. I
don't think players can do that.
Drop the copy of BSD random() we added for Windows.
Like the previous commit, this changes the server's die rolls, and
makes fairland create a different random map for the same seed. Update
expected smoke test results accordingly.
[*] mt19937ar.sep.tgz downloaded from
http://www.math.sci.hiroshima-u.ac.jp/~m-mat/MT/MT2002/emt19937ar.html
"random() % n" is sound only when n is a power of two. The error is
hardly relevant in Empire, because random() yields 31 bits, and our n
are always much smaller than 2^31. Fix it anyway.
Use smallest the 2^m >= n instead of n, and discard numbers exceeding
n.
Bonus: faster for me even in the worst case n = 2^m+1.
Like the recent change to damage(), this changes some of the server's
die rolls, only this time the effect is pretty pervasive. Worse,
fairland now creates a completely different random map for the same
seed. Update expected smoke test results accordingly.
Turns damage() into a one-liner.
damage() now uses random() % 32768 in chance() instead of random() %
100 inline, therefore can round differently for the same pseudo-random
number. Update expected smoke test results accordingly.
Aside: "random() % n" distributes evenly only when n is a power of
two. 100 isn't. However, because random() yields at least 31 bits,
and 100 is so much smaller than 2^31, the error is vanishingly small.
The tech center doesn't have enough workers to use all materials in
some updates. How much get made depends on a die roll then. Tech
variations are inconvenient because they ripple through the rest of
the smoke test.
Too easily upset by random variations. Kill them off with anti after
two updates, and occupy with a few more military.
While there, enlist the military in a highway rather than an lcm
plant.
