Implemented sysCacheClear() #4445

TheAnig · 2018-04-14T20:55:57Z

Implemented sysCacheClear() so that games like Folklore that rely on it don't get stuck on a black screen. Refer to this issue.

Megamouse · 2018-04-14T21:00:24Z

rpcs3/Emu/Cell/Modules/cellSysutil.cpp

+	const std::string& cache_id = param->cacheId;
+	const std::string& cache_path = "/dev_hdd1/cache/" + cache_id + '/';
+
+	if (!fs::exists(vfs::get(cache_path)) || !fs::is_dir(vfs::get(cache_path))) {


we use { in a new line

also you might as well use the vfs::get earlier when initializing cache_path

Alright I'll fix this and post another.

VelocityRa · 2018-04-14T21:05:10Z

rpcs3/Emu/Cell/Modules/cellSysutil.cpp

+	const std::string& cache_id = param->cacheId;
+	const std::string& cache_path = "/dev_hdd1/cache/" + cache_id + '/';
+
+	if (!fs::exists(vfs::get(cache_path)) || !fs::is_dir(vfs::get(cache_path))) {


more of a nit, but you're calling vfs::get on the same path thrice, could make that a separate local.

I'll do this also along with the changes Megamouse proposed.

VelocityRa · 2018-04-14T21:57:25Z

Actually this appears to be incorrect, the function receives no arguments:

As you can see here, r3 is immediately overwritten with the value 1, discarding any arguments that could have been passed.

The CellSysCacheParam is most likely used with the function cellSysCacheMount instead.

VelocityRa · 2018-04-14T21:59:35Z

Did you actually test it? What is passed as the argument?

TheAnig · 2018-04-14T22:05:57Z

I tested a wrong version. I'm working on a fix. (Also I needed game_id subfolder to clear which is what I assumed was the argument)

money123451 · 2018-04-14T22:09:35Z

this PR regresses echochrome NPUA80134 logs
PR
Master

VelocityRa · 2018-04-14T22:10:49Z

As I said, it's wrong, don't test, yet

VelocityRa · 2018-04-14T23:57:15Z

@TheAnig In case it wasn't clear: you are supposed to use fxm::get to get the parameters, instead of the argument.

VelocityRa · 2018-04-15T12:51:55Z

rpcs3/Emu/Cell/Modules/cellSysutil.cpp

 {
-	cellSysutil.todo("cellSysCacheClear()");
+	// Checks if the param exists, else we quit

 	if (!fxm::check<CellSysCacheParam>())


As I told you on Discord, remove this and add it below if fxm::get fails

VelocityRa · 2018-04-15T12:53:07Z

rpcs3/Emu/Cell/Modules/cellSysutil.cpp


-	// TODO: Write tests to figure out, what is deleted.
+	fs::remove_dir(dir_path);


You're supposed to use fs::remove_all(dir_path, true) for it to be recursive.

Alright I'll fix it

TheAnig · 2018-04-15T19:30:31Z

Finally, everything seems fixed. I'll wait for a proper review though, but my intention of fixing Issue #4272 seems to has been realized.

Megamouse · 2018-04-15T19:40:11Z

rpcs3/Emu/Cell/Modules/cellSysutil.cpp

+	// Get the param as a shared ptr, then decipher the cacheid from it
+	// (Instead of assuming naively that the param is passed as argument)
+	std::shared_ptr<CellSysCacheParam> param = fxm::get<CellSysCacheParam>();
+	const std::string& cache_id = param->cacheId;


you still need to check for
if (!param) return CELL_SYSCACHE_ERROR_ACCESS_ERROR;
or something like that

We do that check above in

if(!fxm::check<CellSysCacheParam>())

and return CELL_SYSCACHE_ERROR_NOTMOUNTED

This check was already part of the repo beforehand so I didn't change it

But I told you to...
Also - you can change existing code you know :p

IllusionMan1212 · 2018-04-15T23:56:32Z

PR works as intended, no noticeable regressions.

…hanges in behaviour)

Nekotekina · 2018-04-16T11:15:38Z

rpcs3/Emu/Cell/Modules/cellSysutil.cpp

+		return CELL_SYSCACHE_ERROR_ACCESS_ERROR;
+	}
+
+	const std::string& cache_id = param->cacheId;


Don't bind references, it's old style.

Nekotekina · 2018-04-16T11:16:21Z

rpcs3/Emu/Cell/Modules/cellSysutil.cpp


-	// TODO: Write tests to figure out, what is deleted.
+	// Unit test for param ptr, since it may be null at the time of get()
+	if (!param)


It's pretty much NOTMOUNTED error, fxm::check becomes unnecessary.

Nekotekina · 2018-04-16T11:16:46Z

rpcs3/Emu/Cell/Modules/cellSysutil.cpp

 {
-	cellSysutil.todo("cellSysCacheClear()");
+	// Checks if the param exists, else we quit


Log command shouldn't be removed.

Okay. I'll push a new version tonight after I get back from work.

TheAnig · 2018-04-17T02:57:55Z

Added the requested changes. Everything seems to work.

elad335 · 2018-04-17T04:14:23Z

rpcs3/Emu/Cell/Modules/cellSysutil.cpp

 	// Get the param as a shared ptr, then decipher the cacheid from it
 	// (Instead of assuming naively that the param is passed as argument)
 	std::shared_ptr<CellSysCacheParam> param = fxm::get<CellSysCacheParam>();

+	cellSysutil.todo("cellSysCacheClear()");


use warning log type instead of todo

also.. put it in the beginning of the function

Rpcs3tester · 2018-04-18T16:30:40Z

i think this fix black screen after delete game shader cache .. before it get stuck you need to Reopen RPCS3 to get next screen .

tested twice on Heavenly sowrd

Implemented sysCacheClear()

ecc0cd5

Megamouse reviewed Apr 14, 2018

View reviewed changes

VelocityRa reviewed Apr 14, 2018

View reviewed changes

Fixed paramterization of sysCacheClear() and got rid of memory error

11522bc

VelocityRa suggested changes Apr 15, 2018

View reviewed changes

Changed the remove_dir() call to remove_all() for recursive deletion

7919122

Megamouse reviewed Apr 15, 2018

View reviewed changes

Merge branch 'master' into master

0efa6c9

TheAnig added 3 commits April 16, 2018 02:31

Merge branch 'master' into master

13334f1

Added missing unit test for param (Quality of life change, no major c…

3b6ba7e

…hanges in behaviour)

Merge branch 'master' of https://github.com/TheAnig/rpcs3

472e233

Nekotekina requested changes Apr 16, 2018

View reviewed changes

TheAnig added 2 commits April 16, 2018 21:18

Added changes requested by Nekotekina

d20c7b5

Merge branch 'master' into master

243816d

elad335 reviewed Apr 17, 2018

View reviewed changes

TheAnig added 3 commits April 17, 2018 10:26

Added warning() log at the beginning of the function

b20f750

Merge branch 'master' of https://github.com/TheAnig/rpcs3

5988d5f

Merge branch 'master' into master

0b33e0c

Nekotekina merged commit eb3dfb6 into RPCS3:master Apr 18, 2018

Rpcs3tester mentioned this pull request Apr 20, 2018

cellPad: check for more invalid parameters #4419

Merged

Kravickas mentioned this pull request Apr 21, 2018

Regressions after #4445 #4470

Closed

TigiHof mentioned this pull request May 5, 2018

SPU improvements [test only ASMJIT] #4504

Merged

AniLeo mentioned this pull request May 20, 2018

Folklore [BCES00050] have to delete cache folder in order to boot. #4272

Closed

TigiHof mentioned this pull request Jun 1, 2018

memory access violation [BCUS98298] Beyond: Two Souls #4688

Closed

jeff3779 mentioned this pull request Sep 10, 2019

Game not booting due to leftover dev_hdd1 cache #6526

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented sysCacheClear() #4445

Implemented sysCacheClear() #4445

TheAnig commented Apr 14, 2018

Megamouse Apr 14, 2018

Megamouse Apr 14, 2018

TheAnig Apr 14, 2018

VelocityRa Apr 14, 2018

TheAnig Apr 14, 2018

VelocityRa commented Apr 14, 2018

VelocityRa commented Apr 14, 2018

TheAnig commented Apr 14, 2018 •

edited

money123451 commented Apr 14, 2018

VelocityRa commented Apr 14, 2018

VelocityRa commented Apr 14, 2018

VelocityRa Apr 15, 2018

VelocityRa Apr 15, 2018

TheAnig Apr 15, 2018

TheAnig commented Apr 15, 2018

Megamouse Apr 15, 2018

TheAnig Apr 15, 2018

VelocityRa Apr 16, 2018

IllusionMan1212 commented Apr 15, 2018

Nekotekina Apr 16, 2018

Nekotekina Apr 16, 2018

Nekotekina Apr 16, 2018

TheAnig Apr 16, 2018

TheAnig commented Apr 17, 2018

elad335 Apr 17, 2018

Megamouse Apr 17, 2018

TheAnig Apr 17, 2018

Rpcs3tester commented Apr 18, 2018


		// TODO: Write tests to figure out, what is deleted.
		fs::remove_dir(dir_path);

Implemented sysCacheClear() #4445

Implemented sysCacheClear() #4445

Conversation

TheAnig commented Apr 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VelocityRa commented Apr 14, 2018

VelocityRa commented Apr 14, 2018

TheAnig commented Apr 14, 2018 • edited

money123451 commented Apr 14, 2018

VelocityRa commented Apr 14, 2018

VelocityRa commented Apr 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheAnig commented Apr 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IllusionMan1212 commented Apr 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheAnig commented Apr 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rpcs3tester commented Apr 18, 2018

TheAnig commented Apr 14, 2018 •

edited