q55x8x / Peronality-Creator

Sort of DevEnvironment for teaseAI personalities
4 stars 1 forks source link

Opening lone scripts not working anymore #27

Closed png2 closed 9 years ago

png2 commented 9 years ago

I have no idea what you tried to do there since it's a lot more complex than before when you call the OpenFile function but I can't open scripts not attached to a persona anymore

Why did you separate the opening of modules and fragments? And why attaching the files to the persona tree?

q55x8x commented 9 years ago

I am sepereating modules and fragments basically I will be seperating every file with a different opening process because we can then construct other controls inside a tab related to the type of file we want to edit. Basically to do the same visual studio does when opening a bitmap; it doesn't show the bytecode in a text editor it opens a bitmap editing tab. By doing this we are definitely more future proof if 1885 decides to include other filetypes then txt we can easily add support for these.

The seperation between scripts and fragments happened because we want a different saving process for both and while saving we can easily differentiate between both by looking up the type of the tag. The seperation between fragments and fragmented scripts though just seemed natural to do not sure if we have to keep that.

Why you aren't able to open lone scripts anymore though is a mystery to me it still worked yesterday. I will look into that later. Also we can remove that the lone script is being added to the project view If you think it goes against the logic wasn't sure about it myself but I though it would be a nice thing to have quick access to it if you close it by accident.

png2 commented 9 years ago

Oh ok

Over the years I became a proponent of "don't build for the future until the future become the present" since I realized that the future we imagine rarely become the present we end up leaving but I see what you tried to do

Also the opening probably doesn't work because you don't have any fallback if the file type isn't recognized

I'll refactor this part a bit, you can take a look on it later to see if it still suits you

As for opening a closed file, it would probably be better in a "recently opened files" entry in the main menu than adding orphans in the tree which don't really makes sense for me. What do you think about it ?

q55x8x commented 9 years ago

Well while writing that code it just seemed a good idea to do that as we already need to differentiate between two save processes.

For the recently opened files ... well don't know why I didn't do it that way all along it really seems like the much more logical way

q55x8x commented 9 years ago

fixed in 8a1b00cb86d534aa190a8416f97d56e447cd97fb although to separate the opening process between file types here when we start using other editors to edit e.g. a vocab file we will have to reconsider this patch as soon as they can't be differentiated by their file extension anymore.

EDIT:

Fix was done wrong. 2847f7e460dd22a197c8c3177a4a9583a2cf11c5 fixes it the right way now and we won't have to worry about file type recognition in the future.

png2 commented 9 years ago

I proposed a rewrite on the branch fileOpeningRework based on an object approach and by extracting all the stuff linked to the content of the tab from the main frame

If you want to add a new type of tab, you have to implement the OpenableFile abstract class (done by default by PersonaFile that is still abstract and by Script that implement the current editor)

That means that each file type contain all the stuff needed to make it work in the tab system and that the main frame doesn't care what type of file it manipulates

For now the factory that build the files is still in PersonaFile, at one point it might be a good idea to extract it in a dedicated class since not all files might be implement PersonaFile

Let me know what you think of it, if you are ok with it I'll merge it in the master otherwise we can brainstorm on other implementations

q55x8x commented 9 years ago

this actually looks like a definitely better approach I will pull this branch and have a closer look but at first glance: I like ;)

q55x8x commented 9 years ago

I definitely like this change. I think the re-write there could be a major improvement :+1: But I think we should go ahead and open a new issues for that ;)

png2 commented 9 years ago

I'll just merge with the master once I'm done eating

If you are doing something I recommend you to push before I'm done or the merge will be painful :)

I'll be back in half an hour so that let you some time if you are still around :)

png2 commented 9 years ago

Alright it's merged and looks fine, just recheck all your changes are there just in case because this one was hard to merge

q55x8x commented 9 years ago

Looks like everything went fine I really like this change :+1: