1353 replies to this topic

[toxie]

Thanks for the quick update build!

 

So, as Fuzzel said: Please test this one like crazy, as it changes the triggering of scripts/physics. In theory it should all work roughly the same as before, except for less flipper lag (on the average) due to the more precise timers and the new PinMAME timer handling.

 

Note that using the new special interval setting of -1 for timers is a mixed bag: It will be (currently) triggered 4x per rendered frame, so running at 4x FPS. So its not really a timer in the classical sense then anymore. So, please try to stick with the old setting of interval >= 1 for "normal" timers.

Edited by toxie, 04 November 2016 - 11:48 AM.

[unclewilly]

So putting the pinmame timer at -1 will improve the flipper lag without the scripted workaround?

[toxie]

it should.

but it can by principle never be as fast as the workarounds (as it always has to wait for the emulation to process the input).

[DJRobX]

So putting the pinmame timer at -1 will improve the flipper lag without the scripted workaround?

 

It ought to provide some improvement, but there's more that can be done on the VPM side.   VPM still waits for a vblank to update solenoids in most cases.   If I can change that to be more immediate, it should be a big improvement, especially if Toxie's change now has VP polling/updating VPM at 240hz for a 60hz frame rate.   

[toxie]

its not exactly evenly distributed during one frame, so it depends on the table and the PC it runs on. but when i measure things (keydown and then getting the actual solenoid callback) its at least pretty constant now that one gets only 1 frame delay.

[toxie]

as for the CCC problems: carny/ninuzzu did fixes in the VP-CCC github (https://github.com/C...Priest/CCCforVP), this resolves all issues in combination with the new VP build/core.vbs.

[DJRobX]

On my laptop I found a case (Bodydump's NGG) that hangs my laptop with the new core/timer, but works fine if I change the pinmame timer to 3. 

 

When I run it in a debugger, I can see that it gets stuck in an endless loop of adding timers.   

 

 if(m_script_period <= 1000*MAX_TIMERS_MSEC_OVERALL) // if overall script time per frame exceeded, skip

      {

         const unsigned int p_timeCur = (unsigned int)((m_curPhysicsFrameTime - m_StartTime_usec) / 1000); // milliseconds

 

int size = m_vht.Size();

 

m_vht.size() is up to 75,000 and counting  

[shadowshd]

A big thank you guys 

[toxie]

On my laptop I found a case (Bodydump's NGG) that hangs my laptop with the new core/timer, but works fine if I change the pinmame timer to 3. 

 

When I run it in a debugger, I can see that it gets stuck in an endless loop of adding timers.   

 

 if(m_script_period <= 1000*MAX_TIMERS_MSEC_OVERALL) // if overall script time per frame exceeded, skip

      {

         const unsigned int p_timeCur = (unsigned int)((m_curPhysicsFrameTime - m_StartTime_usec) / 1000); // milliseconds

 

int size = m_vht.Size();

 

m_vht.size() is up to 75,000 and counting  

 

WTF!!!!

I guess this is a more advanced bug that this new scheme is just revealing now.

[DJRobX]

So I did a little more looking into this.   

 

In core.vbs, the pinmame timer seems to be trying to do a sort of "mutex".  There is:

 

Sub PinMAMETimer_Timer

...

Me.Enabled = False

...

Me.Enabled = True

 

...

End Sub

 

Since the timers are being checked by a single loop, and this is a single threaded application, these shouldn't be necessary as there's no way the loop is going to re-enter itself.   If I remove those, the game works fine.

 

The loop that iterates through timers iterates through m_vht, so modifying the timers inside a timer is somewhat dangerous, since the timer routine can modify the contents of the vector while you're iterating through it.

 

 for (int i = 0; i < m_vht.size(); i++)

         {

            HitTimer * const pht = m_vht.ElementAt(i);

            if ((pht->m_interval >= 0 && pht->m_nextfire <= p_timeCur) || (pht->m_interval < 0 && first_cycle))

            {

               pht->m_pfe->FireGroupEvent(DISPID_TimerEvents_Timer);  /// <-- I can modify m_vht! 

 

If I copy the vector first:

 

 Vector<HitTimer> vht;

 

for (int i = 0; i < m_vht.size(); i++)

vht.AddElement(m_vht.ElementAt(i));

 

 

         for (int i = 0; i < vht.size(); i++)

 

It stops the vector from growing out of control, but the game still doesn't run right unless I remove the set/clear timer calls from core.vbs.

 

(Sidenote: MFC/ATL's vector class's copy operator/constructor doesn't seem to work like STL's, it didn't work unless I manually copied the array). 

Edited by DJRobX, 05 November 2016 - 04:17 PM.

[toxie]

good catch.. the first one is really tricky and unnecessary, too, as you say, so i'll remove that..

as for the second one: the balls are doing something similar (as these can be destroyed, too, by the script), so i'll copy the scheme from there..

 

thanks again for the investigation..

[shadowshd]

Hi,
 
Quick feedback with the new CCC code :
 
This morning I copied my old user_settings.yaml over the new one.

When I lost 2 balls in Gold Mine multiball mode, the multiball didn’t end as it should, I was still in multiball mode.

Tonight, I have put back the original user_settings.yaml from the archive and modified it to enter my DMD coordinates.
The multiball ended normally when I lost 2 of the 3 balls.

Then I got the Drunk multiball : when I lost all of the 4 balls, the game got stuck here and didn’t register the fact I’ve lost all balls and didn’t enter the “end-of-ball sequence”.

I then put back core.vbs 10.1 and the game ran smoothly without issues (I did play once, it may be pure chance).

 

++

[Carny_Priest]

Did you lose those balls simultaneously? May have drained too quickly together to trigger.

 

ninuzzu has released a beta to address this.

Edited by Carny_Priest, 05 November 2016 - 08:43 PM.

[toxie]

@DJRobX: I now went for the paranoia solution and tracking all timer changes instead of directly dis/enabling them, and then doing the combined changes before the next timer runs.

 

Scary that this is a bug that was there for ages, and even -always- triggered by the VPM timer itself!!

[DJRobX]

@DJRobX: I now went for the paranoia solution and tracking all timer changes instead of directly dis/enabling them, and then doing the combined changes before the next timer runs.

 

Scary that this is a bug that was there for ages, and even -always- triggered by the VPM timer itself!!

 

Yep I think that's the right approach.   Tracking and applying the change after the loop runs is only truly proper fix I could think of also.   I am also surprised that it didn't cause more issues, as changing timer states is pretty standard in timer handlers.  In the PinMame timer case, it would always cause the next timer in the list to be skipped (because the current pinmame timer will be removed and added to the end every time it runs, with the enable/disable that was in core.vbs).    I guess because the list keeps shuffling around as timers get added and removed stuff got called eventually. 

[toxie]

normally, the PinMAME timer would end up at the end directly after the first run, so if this is the only timer that does that, then its fine after that.

[DJRobX]

But timers enable / disable themselves and other timers all the time.

 

I see an issue with this solution on NGG.    

 

VPM Timer updates a motor callback (lets call it UpdateWheel for clarity).  There's a timer loop that draws a "blurry" wheel if its over a certain speed.  So when the speed drops below 75, we disable the blur timer and restore the non-blurry image map. 

 

Now, the blur timer isn't actually getting disabled until after the loop, so it may run once after being disabled.  If that happens, it leaves the blurry wheel behind, even though the code turned the blur timer off, and then updated the image.   It's easy to code around this but that was not the behavior before, so it might break other things. 

 

Fortunately there's a really easy fix, in timer.cpp, just set the next runtime to UINT32_MAX if it was disabled, so it doesn't get caught in the loop later.   We know it will be scheduled to run at a proper time if enabled again: 

 

g_pplayer->m_changed_vht.push_back(too);
if (fNew)

   m_phittimer->m_nextfire = g_pplayer->m_time_msec + m_phittimer->m_interval;

else
   m_phittimer->m_nextfire = UINT32_MAX;

Edited by DJRobX, 06 November 2016 - 12:25 AM.

[DJRobX]

There's another bug that took me considerably longer to figure out - some timers were not honoring the interval after the first try.  That's because the nextfire setting was trapped in the if (!found) condition.   So the fully fixed version looks like this, and needs to be in both ieditable and timer.cpp.   I had only noticed it in timer.cpp originally and didn't understand why it wasn't working for the longest time.    

 

       if (!found)

       {

         TimerOnOff too;

         too.enabled = !!fNew;

         too.m_timer = m_phittimer;

         g_pplayer->m_changed_vht.push_back(too);

       }

  if (fNew)

  m_phittimer->m_nextfire = g_pplayer->m_time_msec + m_phittimer->m_interval;

  else

 

  m_phittimer->m_nextfire = UINT32_MAX;

 

 

This also means that I think you forgot to clear the change queue after you apply it....  <looks at code> .. Yep.  Probably for the best, the issue may have seemed really obscure had the change queue not been left behind and the problem reproduce consistently.

 

m_changed_vht.clear();

Edited by DJRobX, 06 November 2016 - 04:44 AM.

[toxie]

d'oh, indeed. did this in a rush, so thanks for fixing it.

[shadowshd]

Did you lose those balls simultaneously? May have drained too quickly together to trigger.
 
ninuzzu has released a beta to address this.

 
I'll give a try this morning and report ASAP
 
Edit : I just played 3 times and noticed a new issue :
 
Sometimes when the ball enters the mine, the game gets stuck here and launches a ball search mode.
I let it like this for about 1 minute and got 3 ball search sequences, but the ball didn’t get ejected.

 

I then played with core.vbs 10.1 and the game ran with no issue at all

 

Thanks

 

++

Edited by shadowshd, 06 November 2016 - 11:02 AM.