Refactored tools so that they don't reference `PintaCore` directly

PintaProject / Pinta

Simple GTK# Paint Program

http://www.pinta-project.com/

MIT License

1.74k stars 267 forks source link

Refactored tools so that they don't reference `PintaCore` directly #870

Closed Lehonti closed 3 weeks ago

Lehonti commented 3 weeks ago

This is an effort to get rid of globals.

A few considerations:

Sometimes, methods that aren't part of the interface of IWorkspaceService were called. Luckily, they were simple enough that I could make them into extension methods of the interface itself.
static members of the tools I refactored still reference PintaCore. One example is the TextTool.CurrentTextBounds property. I think these static members don't belong in their current classes, but somewhere else. However, this is a topic for a future pull request.

Lehonti commented 3 weeks ago

It just crashes when I try to use it...I will close it for now. Is this more or less the reason why PintaCore references are still there?

cameronwhite commented 3 weeks ago

It just crashes when I try to use it...I will close it for now. Is this more or less the reason why PintaCore references are still there?

I don't know of any specific reason like that - what crashes were you seeing? I can't seem to clone the PR branch since it was deleted

Lehonti commented 3 weeks ago

Sorry for not showing the error output. For some reason, it couldn't just be copied and pasted, but I should have shown the screenshots at least, in order for the things I observed to be more useful.

GitHub doesn't let me re-open this pull request, but I re-uploaded the branch with the changes to my fork so that you are able to clone it:

https://github.com/Lehonti/Pinta_fork/tree/refactoring/remove_pinta_core_references

cameronwhite commented 3 weeks ago

Thanks, I got an exception in BaseEditEngine constructor from services being null Tracing back a bit, I think the problem is that ShapeTool's constructor calls CreateEditEngine() (an abstract method), and the implementation in LineCurveTool uses the services member. But at that point LineCurveTool.services isn't initialized yet since it's still constructing the base class. Calling a virtual method in a constructor is definitely questionable...