Add ScreenshotEvent

[shadowfacts]

Adds ScreenshotEvent.Pre and ScreenshotEvent.Post which are fired respectively before and after screenshots are taken.

Pre can be used to change the screenshot file and cancel the event.

Post can be used to perform actions on the screenshot after it has been saved.

This was tested successfully using:

import net.minecraftforge.client.event.ScreenshotEvent;
import net.minecraftforge.common.MinecraftForge;
import net.minecraftforge.fml.common.Mod;
import net.minecraftforge.fml.common.event.FMLPreInitializationEvent;
import net.minecraftforge.fml.common.eventhandler.SubscribeEvent;

import java.io.File;

@Mod(modid = "ScreenshotEventTest")
public class ScreenshotTest
{

    @Mod.EventHandler
    public void preInit(FMLPreInitializationEvent event) {
        MinecraftForge.EVENT_BUS.register(this);
    }

    @SubscribeEvent
    public void onScreenshotPre(ScreenshotEvent.Pre event) {
        event.setScreenshotFile(new File(event.getScreenshotFile().getParentFile(), "Screenshot_test.png"));
    }

    @SubscribeEvent
    public void onScreenshotPost(ScreenshotEvent.Post event) {
        System.out.println("Screenshot saved to: " + event.getScreenshotFile().getName());
    }

}

[williewillus]

Maybe provide the BufferedImage if people want to change/use that for whatever reason?

[LexManos]

You can make this 2 lines, and so much cleaner by moving code to the forge class.

[shadowfacts]

@LexManos I don't see how it would be possible to move all that code into ForgeClientHooks. As far as I can tell, all the code that can be in there, is. I need 4 things from the pre event:

• If it's canceled
  • If so, the cancellation message
• The File to use
• The BufferedImage

There's no way to return all three things from ForgeClientHooks.onScreenshotPre.

[kashike]

The indentation is weird throughout this PR

[shadowfacts]

Fixed the indentation issues and a couple incorrect braces that I missed before.

[LexManos]

Humm, im not quite sure why you need to return a completely new buffered image from the event.
You should really only have one event tho.
It should take in a reference to a buffered image. let people manipulate it and move on.
You should also send in and let people modify the file path HOWEVER they should be able to set it to null and disable the whole 'save to file' feature.

Something along the lines of:
file = Forge.onScreenshot(image, file); if (file == null) return;

Not quite sure why it needs a text return but meh.

[shadowfacts]

cancelReason is required because that method returns an ITextComponent and if I returned null on cancellation, it would cause a NPE to be thrown later on because MC isn't expecting null.

[shadowfacts]

Also, yes it could be simplified to one line but this:

net.minecraftforge.client.event.ScreenshotEvent.Pre preEvent = net.minecraftforge.client.ForgeHooksClient.onScreenshotPre(bufferedimage, file2); if (preEvent.isCanceled()) return preEvent.getCancelReason();

is very messy and difficult to read.

[shadowfacts]

I've removed setImage and simplified the patch down to 3 lines total.

[kashike]

This can be final now that the setter is gone.

[shadowfacts]

@kashike image is now final

[LexManos]

Github killed my newline, the if was supposed to be on a separate one.
The point is, we try and keep the patches as small as possible, AND we try and keep as few references to Forge classes from the patches.

And again, i'm still not even convinced that there needs to be a pre/post event to this. Not EVERY event needs a pre/post.

[shadowfacts]

@LexManos The reason I made it this way is my ShadowTweaks mod adds a couple of things pertaining to screenshots:

1. The ability to change the screenshot directory (this requires an event to be fired before the screenshot is saved in order to change the file)
2. The ability to upload a screenshot to Imgur after it's taken (this requires an event to be fired after the screenshot is taken and saved)

If you have a way to make both these work, I would gladly use that, but this is the only solution I've been able to come up with.

[LexManos]

Fire one event.
To Change the file path: Change the file path
To Upload the image: Encode the bufferedimage and upload it from MEMORY not file.
If you want you could prevent it saving to file by setting the path to null.

[shadowfacts]

I rewrote it to only use one event which reduced the path to 2 lines.

This code was used to test it:

import net.minecraft.util.text.TextComponentString;
import net.minecraftforge.client.event.ScreenshotEvent;
import net.minecraftforge.common.MinecraftForge;
import net.minecraftforge.fml.common.Mod;
import net.minecraftforge.fml.common.event.FMLPreInitializationEvent;
import net.minecraftforge.fml.common.eventhandler.SubscribeEvent;

import java.io.File;

/**
 * @author shadowfacts
 */
@Mod(modid = "ScreenshotEventTest")
public class ScreenshotTest
{

    @Mod.EventHandler
    public void preInit(FMLPreInitializationEvent event) {
        MinecraftForge.EVENT_BUS.register(this);
    }

    @SubscribeEvent
    public void onScreenshotPre(ScreenshotEvent event) {
//        event.setScreenshotFile(new File(event.getScreenshotFile().getParentFile(), "Screenshot_test2.png"));
        event.setCancelReason(new TextComponentString("Testerino"));
        event.setCanceled(true);
    }

}

[BlayTheNinth]

Why is it specifically a "CancelReason" and not just a "ResultMessage" or something that gets returned in place of the actual message as long as its not null? I can see myself needing that if I were to implement TwelveIterationMods/EiraIRC#343 ([Upload & Share] Link in Screenshot Message) without hacks.

... of course I could just cancel it and then perform the ImageIO save-to-file myself, but I feel it'd be cleaner to just widen cancelReason to a general result chat component.

[shadowfacts]

Added ScreenshotEvent#resultMessage

[shadowfacts]

Anything else?

[shadowfacts]

Replaced by #2828.