andanteyk / ElectronicObserver

艦これ補助ツール「七四式電子観測儀」
MIT License
347 stars 209 forks source link

RequestHandler in feature-cef #234

Open xiaozhikang0916 opened 6 years ago

xiaozhikang0916 commented 6 years ago

Found that a request filter is implmented to enable the setting preserveDrawingBuffer but I get some questions here:

  1. What is the purpose of this setting, game view capturing and streaming?
  2. It may cause a great performance drop in game, is it necessary to suffer such performance drop, or do we have other way to achive what mentioned in 1
  3. Is it a break to the promise Will not manipulate the communication between client and Kancolle server to change the setting by replacing Javascript code in the response
andanteyk commented 6 years ago

Thank you for review.

1: It is to take screenshots. If you take a screenshot with preserveDrawingBuffer = false (default value), you will get a black image.

The requirements of the screenshot function are:

In order to fulfill this requirement, this process is now required. I tried several other methods, but I could not fulfill this requirement. (I am still looking for them)

2: I understand that this setting makes processing heavier. However, in my environment I did not notice delay; so I did not think that it was such a big problem.

3: Regarding that sentence, I mainly assume the KanColle API (like kcsapi/api_port/port). Because this replacement may not affect the main body of KanColle, I adopted this method. Of course, a method that does not require replacement is better. If there is a such method, I will adopt that method.

Do you have any good ideas?

xiaozhikang0916 commented 6 years ago

I also opened an issue in @RadarNyan 's cef browser repo (link) and discussed about the preserveDrawingBuffer setting.

According to RadarNyan's experience, the performance drop of enabling such setting is unacceptable on some low-performance platform.

An alternative way to achieve this requirement is to disabling Hardware Acceleration of the browser, which is also mentioned in the linked issue. But some rendering issues, like being gray of sinked ship and fade-out effect of ship's name, are reported(link for reference). However, I haven't come to this problem.

For 3, I personally feel OK with this method to change setting (if it is necessary) because I can read the code and know what it may or may not affect. But for the common users, it might be better to have a detailed description, especially whether it will manipulate the game api or not; or make it an optional way in the settings.

By the way, have you tested if the game can be recorded as video or live streaming in window mode? I tried build the cef brench but fail to launch EOBrowser

andanteyk commented 6 years ago

But some rendering issues, like being gray of sinked ship

Like this? I could not reproduce it either (both hardware acceleration on/off). It may already have been fixed by KanColle staff ...?

For 3, ... or make it an optional way in the settings.

It is reasonable. It is under consideration.

have you tested if the game can be recorded as video

By external tool? When using the "game bar" function of Windows 10, recording succeeded with either hardware acceleration on or off.

xiaozhikang0916 commented 6 years ago

When I am using Chrome, I cannot capture game view by toDataURL, nor video streaming using OBS in window mode(got a black view) with default settings(Hardware Acceleration enable, preserveDrawingBuffer = false).

But if I disabled Hardware Acceleration (with preserveDrawingBuffer = false as default), it become both enable to get view capture by method toDataURL (tested in Chrome console commend) and video streaming.

So I think it can be an alternative way to implement game capturing if rendering works fine as usual and has not much performance loss. But it needs more further test to figure out if it is better.

andanteyk commented 6 years ago

Memo:

If an instance of PIXI.WebGLRenderer is obtainable, We may be able to take screenshots using the following method, even though preserveDrawingBuffer=false.

// When take_ss is set to true (by other script), screenshot is saved to dataurl

var dataurl;
var take_ss = false;
var canvas = document.querySelector('canvas');

let renderer = /*<instance of PIXI.Application>.renderer or <instance of PIXI.WebGLRenderer>*/;
renderer.on('postrender', function() {
    if (take_ss) {
        take_ss = false;
        dataurl = canvas.toDataURL('image/png');
    }
});

This method worked on other applications using PixiJS. I do not know how to get that instance in KanColle, though.

xiaozhikang0916 commented 6 years ago

Tried in several ways:

1: Hack the main.js of Kancolle, remove all the "use strict;" and save the PIXI.Application in a global varable right after it is inited. Anyway, it works, and it can be simplified like:

However, such method has quite some disadvantages

2: Hack the pixi.min.js or Application.js of PIXI to save an instance after constructor of PIXI.Application is called, but failed:

In my opinion, the instance should be accessable globally without previous hackings since the game is running, but it needs time to study the code. However, previous method can be implemented as a experiment feature if you like to (and if the performance drop of current method is sensitive to users)


It is appriciated to find that you make preserveDrawingBuffer as an optional setting in the latest commit. But would you mind making it as a standalone settings rather than associated with hardware acceleration? Some friends of mine said that they prefer the original game settings even though the capturing is unfuntionable ("Capture can be done by external tools, which is not the main feature of 74EO", as the said)

andanteyk commented 6 years ago

I appreciate the investigation.

I think it is dangerous to overwrite KanColle. KanColle is frequently updated, trouble may occur before we notice it.

Anyway, it is still in the experimental. I think a deeper investigation is necessary to implement that.


At 8de8efe7a469d6a2368879f34e2b2e452f5e347d , I made it possible to specify Hardware Acceleration and preserveDrawingBuffer individually. In addition, if it can not take a screenshot using the original method, it can take with Graphics.CopyFromScreen instead. At least it is better than it stops working.

xiaozhikang0916 commented 6 years ago

Got a new idea.

We are aiming to get a reference of Pixi.Application and leak it right after it is constructed. So let's begin in a simple example.

Let's say we have a class stands for Pixi.Application

class Application {
    constructor(server) {
        console.log("You are playing Kancolle in " + server)
        this.server = server
    }
    printServer() {
        console.log("Your server is " + this.server)
    }
}

And we are going to rewrite it's constructor, following the guide in stackoverflow, we inject the following code

BackUpApplication=Application
BackUpPrototype=Application.prototype
Application=function(server) {
    console.log("Opps, your game has beek hacked")
    return new BackUpApplication(server)
}
Application.prototype=BackUpPrototype

Then, if the constructor of Application is called as usual, you will get

var user = new Application("sasebo")
//Console ouput : Opps, your game has beek hacked
//Console ouput : You are playing Kancolle in sasebo
user.printServer()
//Console ouput : Your server is sasebo

So, we can load a script like block 2 right after Pixi.min.js is loaded and before the game starts to rewrite the constructor of Pixi.Application and get a global reference of the new constructed instance, without really do a rewrite on the script code.

However, would you mind to try implementing it in a demo? I seem not likely have such a time to make a runnable demo (Previous code is tested in Chrome console)


Update: Maybe we don't really need a global reference, but just do the registration of onpostrenderer in the hacked constructor.

xiaozhikang0916 commented 6 years ago

Seems that I'm heading a damn wrong way.

A friend of mine reminds me that requestAnimationFrame is the best way to capture a game view, without changing setting of preserveDrawingBuffer, nor changing the constructor of PIXI.Application.

And that is what 74EO is doing now. So it means you can just remove the RequestHandler safely. The screen-shot feature will be still functionable, even with Hardware Acceleration enabled and preserveDrawingBuffer=false


Removment is tested on my build, capturing works fine

andanteyk commented 6 years ago

I also confirmed that it will work. Thank you for your information. I will fix it on the next release.

xiaozhikang0916 commented 6 years ago

It seems that you are feeling confused why it works even preserveDrawingBuffer=false and HA=true.

With previous settings, browser is using WebGL to draw image to canvas, whose buffer will be clear after drawing is finished (in other words, flush action is done). And calling toDataURL from console or CEF browser can be seen as an async action, it's calling timing is not guranteed. So if you have the luck to call it before clear is called, you can get the view, otherwise, a black image. And changing preserveDrawingBuffer to true means that the buffer will never cleared before next frame is pushed to buffer, so we can get valid image from the buffer at any time we want.

Here is a Chinese blog for reference

But now we are calling through requestAnimationFrame, which is designed to requests that the browser call a specified function to update an animation before the next repaint (description from mozilla), so our request of calling toDataURL can be called before drawing buffer is cleared.

Hope I explain this clearly.

xiaozhikang0916 commented 6 years ago

Sorry to reopen it, but since screenshot works fine now, is it necessary to have an option of preserveDrawingBuffer in settings? Does it have any other purpose?