PCSX2: Avoid enabling VuClipFlag at some scenarios

[ssakash]

Summary of changes:

• VuClipFlag hack is only used by the SuperVU1 Recompiler, let's avoid enabling it when it's not used.

Previously a console message would be provided stating that (GameDB) Enabled Gamefix: VuClipFlagHack even when the user is using the Interpreter (or) MicroVu Recompiler. The following patch prevents such confusions.

[FlatOutPS2]

But what if you switch recompilers while playing one of games that need the fix?

[ssakash]

But what if you switch recompilers while playing one of games that need the fix?

I assume you mean from MicroVU -> SuperVU ? If that's the case, the game should still work fine as the gamefix is instantly enabled after detection of the recompiler switch.

[refractionpcsx2]

That will enable it if you are using the interpreter (unlikely i know, but as likely as it being enabled having an effect on MVU :P ) , maybe rethink your if statement, possibly to match supervu rather than microvu :)

[ssakash]

That will enable it if you are using the interpreter

You're wrong, the gamefix won't be enabled when using the interpreter.

Current code:

if (id == Fix_VuClipFlag && (!g_Conf->EmuOptions.Cpu.Recompiler.EnableVU1 || g_Conf->EmuOptions.Cpu.Recompiler.UseMicroVU1))

Where,
(!g_Conf->EmuOptions.Cpu.Recompiler.EnableVU1) _->_ Interpreter on VU1
g_Conf->EmuOptions.Cpu.Recompiler.UseMicroVU1 _->_ MicroVU on VU1

[refractionpcsx2]

ah yeh, my bad :P

[refractionpcsx2]

That said, does this really need to happen? The flag is only used in supervu, it won't make a difference if you are using any other VU's, so it's technically already off.

[FlatOutPS2]

It might not be necessary for technical reasons, but why show users a fix has been enabled when it's not currently enabled?

[refractionpcsx2]

I didn't realise it was something people got confused about :P I can't say I've seen anybody wondering why it's enabled.

But fair enough.

[FlatOutPS2]

That doesn't really matter. Like you said, the fix is off when SuperVU isn't used, so there's no reason to show that a game fix has been enabled either.

That said, the fix might confuse people who want to know what it actually does. It doesn't say what it fixes in the gameDB or on the game fixes tab.

On a slightly related note: It's "game fixes" in the Emulation Settings menu, but on the system tab it's "Automatic gamefixes"(one word instead of two). We should probably fix that as well. :P

[refractionpcsx2]

so out of curiosity, what happens if they switch renderer, does it enable it still?

[avih]

How about just changing the display name to include SuperVU in it? this way it would be clear that it's irrelevant for MicroVU

[FlatOutPS2]

How about just changing the display name to include SuperVU in it? this way it would be clear that it's irrelevant for MicroVU

What's wrong with this version? It's much clearer for the user as it doesn't show anything until SuperVU is actually used.

so out of curiosity, what happens if they switch renderer, does it enable it still?

Nothing happens, but it does enable and disable when switching recompilers(see my question in the first reply). :p

[avih]

What's wrong with this version? It's much clearer for the user as it doesn't show anything until SuperVU is actually used.

Technically correct, but it could confuse anyone debugging it since the DB has a hack but there's no feedback that it actually gets applied. If instead it said something like "Enabling SuperVU VuClipFlag hack" it would be clearer what's going on IMO.

That being said, there's nothing really wrong with it either way IMO.

[FlatOutPS2]

There is feedback when it gets applied. As soon as you switch to the SuperVU recompiler the fix is mentioned in the log and on the top of the log window as usual.

[avih]

There is feedback when it gets applied. As soon as you switch to the SuperVU recompiler the fix is mentioned in the log and on the top of the log window as usual.

Clearly I meant that it exists at the DB but SuperVU is not enabled hence there's no feedback despite it being at the DB and supposedly (to the uninitiated) should be getting applied but seemingly isn't.

If there's still a message that the SuperVU hack is enabled, it's clear what's going on. It's enabled, but irrelevant since we're not in SuperVU.

[FlatOutPS2]

If someone wants to debug it, they'll likely have read the info in the gamedb or on the game fixes tab where it says it's sVU only, so they should be able to understand that's only enabled when the sVU is used as well.

And we could always add a log message that the fix isn't applied because the user is using another recompiler.

[avih]

If someone wants to debug it, they'll likely have read the info in the gamedb or on the game fixes tab where it says it's sVU only, so they should be able to understand that's only enabled when the sVU is used as well.

And we could always add a log message that the fix isn't applied because the user is using another recompiler.

Or, instead of all those, just rename the display name to include SuperVU and solve everything in one go.

[FlatOutPS2]

No, that will make the uninitiated think a superVU fix has been enabled even though they're using microVU.

[refractionpcsx2]

It might be an idea to check it on the initialisation of the VU's, so on the microVU init it checks it and displays in the console "VUFlag hack enabled but not used with microVU" or something, then SuperVU can have "VUFlag hack detected, enabling" or something, just so there's some feedback to the user.

[FlatOutPS2]

@ssakash 's current commit with a console message ""VUFlag hack loaded, but will only be enabled for SuperVU recompiler." on the line above continue; would really be the best. It'll have the correct behaviour in all circumstances.

[ssakash]

Currently I'm fine with FlatOut's suggestion however before making the changes, I'd prefer to see how #1622 turns out ;)

[refractionpcsx2]

Currently I'm fine with FlatOut's suggestion however before making the changes, I'd prefer to see how #1622 turns out ;)

I wouldn't count your chickens on that one, it's generally an unfavourable opinion. Best start planning your changes :P

[gregory38]

short status for the guy that doesn't want to understand the PR ;) Merge/postponed ?

[refractionpcsx2]

postponed until ssakash stops thinking we're going to drop SuperVU any time soon :P he has a couple of changes to make before merge.

[lightningterror]

I'd actually like to see this merged, it's a more correct behavior. Plus renaming the hack is also a good idea so I suggest doing both of these.

[arcum42]

I'm not particularly sure why this hasn't been merged. It seems like a straightforward change, and well, superVU hasn't been dropped in the last two years. No point in waiting for it.

[refractionpcsx2]

As I said

he has a couple of changes to make before merge.

and he still hasn't made them

[arcum42]

How many changes could there be to make to three lines of code? ^_^

[refractionpcsx2]

I can't remember and too lazy to read the thread, regardless, he hasn't done the changes requested :P

[arcum42]

@ssakash Do you remember what changes you were going to make? Any reason not to just merge this now?

[lightningterror]

If he's fine with it we can finish the pr ourselves. Years passed already ...

[arcum42]

Yeah. I just hate having all these "postponed" pr's sitting around gathering dust. Or non postponed ones that just never get pushed.

[lightningterror]

I vote for closing them and making some documentation in the Projects tab about them.
Otherwise they have no purpose now.

[arcum42]

I suspect we'll have to address them case by case. This one we can probably get merged. Greg's FPU one can be tested now that I rebased it.

His recompiler changes are very complicated to rebase and fix conflicts, and are one of these things I suspect will sit there unless he works on them himself... (though they are probably things that should be done)