Ticket #1193 (new defect)

Opened 15 months ago

Last modified 8 weeks ago

Replace SpiderMonkey's Math functions with platform consistent versions

Reported by: historic_bruno Owned by: quantumstate
Priority: Should Have Milestone: Backlog
Component: Core engine Keywords: spidermonkey
Cc: Philip

Description (last modified by historic_bruno) (diff)

As Philip reported on Bugzilla, there are differences in SpiderMonkey's floating point Math functions on different platforms. Even a small discrepancy can cause OOS errors in multiplayer games, if those functions are used in e.g. simulation, map generation, or AI code. We already use fixed point approximations for sin/cos and atan2 in the engine, but not yet for scripts. Currently no scripts appear to use atan, acos, or asin.

(There may also be differences in pow and exp which we do use rarely, but I wasn't able to reproduce them with the above test cases.)

Attachments

1193.diff (2.6 KB) - added by quantumstate 15 months ago.
These changes should fix the ticket, will wait till after A9 to commit them

Change History

comment:1 Changed 15 months ago by quantumstate

  • Owner changed from historic_bruno to quantumstate

The sin/cos approximations which are accurate to about 10-9 have been added with commit http://trac.wildfiregames.com/changeset/11226 . I will look at writing a good approximation for atan2.

comment:2 Changed 15 months ago by historic_bruno

  • Description modified (diff)
  • Summary changed from Replace SpiderMonkey's trig functions with safe fixed-point versions to Replace SpiderMonkey's trig functions with platform consistent versions

Updated title and description since fixed point is no longer the focus.

Also [11220] is relevant, it introduced a new rmgen function GetAngle, which calls Math.acos. I don't know if acos is even a problem, I simply mentioned all the trig functions :)

comment:3 Changed 15 months ago by quantumstate

In 11231:

Added atan and atan2. Refs #1193

comment:4 Changed 15 months ago by historic_bruno

This ticket is probably solved well enough for Alpha 9. Would be nice to add some tests eventually.

comment:5 Changed 15 months ago by k776

  • Milestone changed from Alpha 9 to Alpha 10

Changed 15 months ago by quantumstate

These changes should fix the ticket, will wait till after A9 to commit them

comment:6 Changed 13 months ago by k776

Bump. Can we get this finished up easily?

comment:7 Changed 13 months ago by leper

A slightly modified version of the last diff was applied in [11363].

comment:8 Changed 13 months ago by quantumstate

This is not closed because we were wanting to add some tests. I should get round to this sometime soon.

comment:9 Changed 13 months ago by k776

  • Milestone changed from Alpha 10 to Backlog

comment:10 Changed 12 months ago by historic_bruno

I would suggest that as described in #433, the Math tests be run at game startup by everyone, not only in the standard test suite. That way we can hopefully detect problems like [11857] sooner and with more explicit errors.

comment:11 Changed 11 months ago by ben

In 12056:

Implements global tech modification function. Fixes #1358, refs #1520.
Applies tech modifications to template data returned by GuiInterface?.
Extends engine to load arbitrary global scripts, separates this from RNG replacement. Refs #1193.
Loads global scripts for most script contexts for consistency.
Adds simulation tests for global scripts.

comment:12 Changed 8 weeks ago by historic_bruno

  • Summary changed from Replace SpiderMonkey's trig functions with platform consistent versions to Replace SpiderMonkey's Math functions with platform consistent versions

Should also replace Math.pow and Math.exp at some point.

Note: See TracTickets for help on using tickets.