View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001050 | FSSCP | physics | public | 2006-09-15 20:08 | 2006-09-23 11:05 |
| Reporter | Backslash | Assigned To | Backslash | ||
| Priority | normal | Severity | tweak | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Summary | 0001050: max_speed is a magnitude of Max Velocity, causing a trichord bug | ||||
| Description | In ship.cpp we have the lines: if(optional_string("$Max Velocity:")) stuff_vector(&sip->max_vel); // calculate the max speed from max_velocity sip->max_speed = vm_vec_mag(&sip->max_vel); The problem we noticed is, what if Max Velocity consists of more than just forward, so sliding speeds as well? max_speed then becomes MORE than the max Z speed velocity, and thus this new value becomes your maximum throttle. (Note the maximum throttle is only applied to the Z axis.) $Max Velocity: 0, 0, 50 shows your maximum forward speed as 50. $Max Velocity: 50, 0, 0 STILL shows your maximum forward speed as 50, even though you have specified 0. (sqrt(50^2+0+0)) $Max Velocity: 50, 50, 50 shows your maximum forward speed as 86 (sqrt(50^3)). Worse, during trichording you can go up to 112! (sqrt(50^2+50^2+86^2) (Of course, this is the _real_ speed you're going... the in-game speed will just show your Z velocity. You might need the Velocity Indicator script (http://www.hard-light.net/wiki/index.php/Script_-_Velocity_Indicator), or custom debug code in the throttle indicator. | ||||
| Additional Information | I posted this in http://www.hard-light.net/forums/index.php/topic,41832.0.html a couple weeks ago. Goober mentioned this was related to an attempt to deal with the trichording bug... however in my opinion this code CAUSES a trichording 'bug'. I'd change this myself right now but I suspect there's some issue I might not be considering, so thought I'd ask about this first. Is there ever a situation where max_speed SHOULD be taking sliding into account? Maybe Mantis will get a bit more attention, since this turns out to manifest as a bug. and hey, this way if I end up fixing it, it's recorded in Mantis as something I fixed :-D | ||||
| Tags | No tags attached. | ||||
|
|
You know, I'm glad you brought this up. I didn't realize it before, but this is the same thing argv noticed three years ago. I saved his email because I wasn't sure what to think of it, but perhaps he was right after all: ----------------------------------------------------------------- In code/ship/ship.cpp, parse_ship(), we have: sip->max_speed = vm_vec_mag(&sip->max_vel); Goober comments: // Goober5000 - it's always worked before, and suppose a ship moves in // multiple dimensions? Ironically enough, this breaks if the ship does move in multiple dimensions. The max_speed field is used only for the max forward speed; max speed in other dimensions and backwards is done elsewhere. Doing a vm_vec_mag here is likely to give a max_speed that is higher than the ship's max oclk speed, in which case increasing the ETS engine power causes max speed to decrease! The code I put here is: sip->max_speed = sip->max_vel.xyz.z; I've tested this with both ships that move in multiple dimensions and those that do not; this gives correct results in both cases. Try it yourself if you don't believe me: make a player ship that can move in multiple dimensions, and a max oclk speed higher than the normal max speed. Play it, and increase the engine power on your ETS to full. Notice how the resulting max speed (the one displayed on your HUD speed gauge) is not the max oclk speed you set in the table, and is probably less than the max speed you got with equal ETS settings. For this reason, I am putting my fix back in for my experimental branch. It may go into HEAD pending resolution of this issue. _argv[-1]. |
|
|
Three years? Wow. So _argv[-1] was a human! ;-P What else did he do? Yeah, that small code change seems to work so far. I'm not noticing any problems, and I'm testing the more complicated things that use max_speed, like autopilot. I'll commit this to HEAD on Sunday if nobody comes up with an objection. Heh, that makes me think... does the AI know how to use craft with zero maximum Z speed and non-zero maximum X or Y speed? This could be funny :-D |
|
|
Why don't you go ahead and test it? ;) I'd like to hear a go/no-go from taylor, but at this point it looks like this fix should go in. |
|
|
I've seen the ETS problem before, but never really looked into it. Nice to finally know what that was. :) I don't have a problem with it going in either branch (assuming that it's well tested before it hits the stable branch at least) since I feel that this will be a rather significant problem with the upcoming mods which make use of this (which is most of them). My only concern is that there almost appears to be more to this than we're seeing. How AI handles this will be something to watch out for since I have a strange feeling that this could do some weird things to balance with Shivan ships in the original campaigns. This is just subtle enough to break balance in a few cases. I'm not sure how to test that theory out though. |
|
|
Exactly why I brought this up, the subtle effects I don't know about. Heh, and in that case you're going to LOVE my next report :-P |
|
|
Heh, maybe I should go look at what else argv was doing. Committed. I'll commit it to 3_6_9 in a few more days (and once I figure out how :-P ) |
|
|
Ugh, I reverted your commit. You had a single change, but that took a 122k, 3,921 line diff to make happen. Please get those excess commit changes in check and re-commit the fix. I'd seriously hate to keep reverting your stuff because of CVS client settings. If you aren't sure what it's going to do then get a diff before you commit and review what it intends to change first. If it's not what you expect, then make the needed changes, and get another diff to verify that it's correct before final commit. |
|
|
Aiee! I apologize, that IS seriously annoying! By all means, revert, and thank you. Thing is, I DID do a diff before committing and it showed me what I expected to see, so I thought I had the problem licked. But now that I look back it seems TortoiseCVS ignores whitespace diff. Bleah... I liked WinCVS better (hahaha!) but it refuses to commit... [sizable gap of time] Ok, after a reinstall, recheckout, and barking up many of the wrong 'trees' (ba-dum tsh), I seem to have fixed the problem (with WinCVS, and thus hopefully the whitespace problems). Shouldn't happen again. Slap me up if it does. |
|
|
Fixered, really this time. Other than how much CPU it takes up even idle, I much prefer the amount of advanced options WinCVS now gives me, especially regarding forcing Unix file format, and whitespace. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2006-09-15 20:08 | Backslash | New Issue | |
| 2006-09-15 21:03 | Goober5000 | Note Added: 0006634 | |
| 2006-09-15 22:44 | Backslash | Note Added: 0006635 | |
| 2006-09-15 23:28 | Goober5000 | Note Added: 0006636 | |
| 2006-09-16 00:20 | taylor | Note Added: 0006637 | |
| 2006-09-16 00:53 | Backslash | Note Added: 0006638 | |
| 2006-09-16 23:29 | Backslash | Status | new => assigned |
| 2006-09-16 23:29 | Backslash | Assigned To | => Backslash |
| 2006-09-20 08:22 | Backslash | Status | assigned => resolved |
| 2006-09-20 08:22 | Backslash | Resolution | open => fixed |
| 2006-09-20 08:22 | Backslash | Note Added: 0006656 | |
| 2006-09-21 12:24 | taylor | Status | resolved => feedback |
| 2006-09-21 12:24 | taylor | Resolution | fixed => reopened |
| 2006-09-21 12:24 | taylor | Note Added: 0006661 | |
| 2006-09-22 08:00 | Backslash | Note Added: 0006667 | |
| 2006-09-23 11:05 | Backslash | Status | feedback => resolved |
| 2006-09-23 11:05 | Backslash | Resolution | reopened => fixed |
| 2006-09-23 11:05 | Backslash | Note Added: 0006682 |