Scripting engine refactoring?

I'm mucking with the CScriptEngine code right now, aiming to refactor it so new scripting interfaces can be defined easily. If anyone has experience with the scripting engine code (or WSH in general) and is willing to share anything they might know, I'd appreciate it. And in case anyone is willing to lend a hand, here's my current battle plan. It's probably going to change a lot, too; this is just something I've built out while hacking on a pseudo-solution in Notepad++.

Initial thoughts: non-WSH Lua support seems to have been hacked into the CScriptEngine class to short-curcuit normal CScriptEngine behavior with regard to WSH, including lots of special-cases like IsLua(), ExecuteLua(), etc. It's basically DIY polymorphism. At first glance it doesn't seem too hard to create a base interface (IScriptEngine), which any scripting engine will have to implement.

In the process, I'm removing CreateScriptEngine() and DisableScripting(), moving their functionality into the constructor/destructor, and using exceptions in the constructor. Changing calling code here was trivial, and I think it looks better too.

Here's the current IScriptEngine interface. Please don't assume this is the final version. This is just what's pseudo-working right now. (I haven't created a LuaScriptEngine yet.)




The biggest design roadblock I've hit - luckily during the Notepad++ phase - is that MUSHclient intentionally treats Lua and WSH differently. This is going to make it really, really, really hard to abstract behind a common interface. For example, Lua functions registered with a trigger will receive the line styles as an extra parameter. This is passed as a table, which I'm not sure WSH can gracefully handle. (VBscript is an obvious example of a language without built-in maps.) Obviously, I can't lock down Lua from doing this. It would be simpler for me XD but it's a non-option.

I have two solutions I've been mulling over. The first is to come up with some common parameter/returns interface that remains backward-compatible and can handle the breadth of options Lua has. Ummm... Just writing that down was disheartening. The only way I've considered to do this is to use a DISPPARAMS structure of VT_ARRAY|VT_VARIANT, which I think can be nested. Backward compatibility is still up in the air at this point.

The second solution I've considered is to have "contextual" methods in IScriptEngine, so the engine knows what your intention is and can adapt the parameters accordingly. This seems most workable, especially as it seems to be how ExecuteLua works currently. (Among its parameters are a PaneLine and a t_regexp...) The idea is that you'd pass everything a given call might want, and the script engine implementation would pass on only the parameters it can handle. This is my preferred solution.

I'd really like to hear feedback on this in particular. It's the biggest issue I have right now.



tl;dr: I'm making lots of changes to scripting support, and I'd like feedback. I also have a pretty good-sized problem. Can you help me?

Thanks for reading! :S

I have a really bad habit of editing my posts long after they're posted, so unless you're just now reading this, you might want to look up again and see the IScriptEngine interface I inserted. =/

Scripting languages that support WSH can already be added easily. Is the goal here to add languages that don't support WSH? Or is the goal to provide native-like embedding such as what Lua has? Is there a language in particular you want to support right now that does not work with WSH?

Designing a system that provides tight coupling with many different languages while remaining loose and agnostic to details will be hard, and essentially a reimplementation of WSH.

That said, if all cases are that calls take either X parameters, or some subset of those X, then it would be easy enough to have a map from language/"WSHness" to the subset of parameters -- especially if it's a prefix rather than a general subset.

BTW, I wouldn't say that eight minutes is "long after" the first post. ;-)

You're right though that it's not a native type in non-.Net.

Both, really. And to clean up the interfaces in general. I'm a big fan of low coupling, as you know. :)

Some of the WSH-based languages have odd differences from their non-WSH counterparts. Ruby is particularly weird in my opinion, because you're supposed to be able to call "def foo; end" if it's defined in the top-level. ActiveRuby requires "def self.foo; end".

And I think it would be cool to support the V8 Javascript engine, which is what powers Google Chrome. It's a very fast implementation of the language, plus it would be usable under Wine.


Yes, and that's what the first solution I described would effectively do. I slept on it, and I don't feel like it would be the best route to take. The context-specific calls would be easier, and it's not like we're out to reuse this outside MUSHclient. I simply need to define a context-specific execute for each time where MUSHclient calls the engine, which is what ExecuteLua already does. The problem with ExecuteLua is that it's specific to Lua and it has special-cased parameters too, but I believe I can probably split those into separate method signatures.


There've been a few situations where someone posted while I was still editing and I didn't think they had seen my edits, so hey, better safe than sorry. :)


[EDIT]: I forgot to mention something I've been reading a lot of lately. I found Bjarne Stroustrup's Style and Technique FAQ [1], and also a style guide [2] that he recommends for C++. I agree with a whole lot of the rules in the latter, except for some like enforcing lowercase constant/enum identifiers on stylistic grounds. But I think they're good to read.

[1] http://www2.research.att.com/~bs/bs_faq2.html
[2] http://www2.research.att.com/~bs/JSF-AV-rules.pdf

The scripting engine is a core component of the client and likely to be rather delicate. As such personally I would be very wary of changing something so major about it unless it was for a very clear goal. Does all of this need to be done to include Javascript V8? How important is it to have that? People seem to get by just fine with Lua and the other languages. Very few people seem to use JScript for the moment, and I doubt it's because of performance issues.

Yes, it would be "cool" to support Javascript V8, but how "cool" will all the time that needs to be spent on this be? I'm saying this not w.r.t. your time because you can do whatever you want with it and if this is fun to you then it's worth it, but w.r.t. Nick's time (and potentially other people) involved in the reviewing, testing, etc.

It's starting to feel like the source code is untouchable. I do some t_regexp refactorings and I'm told new features would be better, so I tackle CScriptEngine and I'm told nobody wants to spend the time to check my changes. So why am I here?

You suggested recently a requirement for rotating images (which you did not provide suggested source for). I agreed that could be useful (although haven't seen it "in the wild" however) but I did it within a couple of days of being asked.

I could see the benefit in this case of spending time on it.

For me, using my time involves a cost/benefit analysis. I'm not talking cash here, but a reward for time spent.

For example, adding miniwindows was a major time expenditure (even yesterday I spent the entire day improving documentation until my eyes went blurry). But the benefits are fairly obvious. MUDs I know of (like Aardwolf, God Wars, Achaea, et. al.) are using it to good effect to provide maps, gauges, chat logs, XP bars etc.

But rewriting the regular expression handler internally? This wouldn't have any tangible benefit to players. Or redoing the script engine? Now I am the first to admit that when I added scripting I hadn't heard of Lua, and didn't know much about the WSH. So it is really crappy, I admit that. But despite that, it seems to work. I get few complaints, if any, that simple scripting has major flaws or crashes.

I don't quite get why you want to add Javascript V8 (whatever that is). I thought you, Twisol, personally loved Lua. Are you bored with it?

It's a bit like Jumbo Jet designers wanting to provide 240v power, 110v power, 50 Hz and 60 Hz, NTSC and PAL on their jets for their customer's use "just in case" someone has a shaver or laptop charger that doesn't work with the supplied standard. But, people accept the limitations and work around them.

I have said time and time again that I know the source can be improved. It could be rewritten to get rid of MFC and then you could compile with g++. You could change all the MFC lists and maps to STL ones. You could improve the graphics. Scripting could be reworked. The GUI interface could be modernized and improved. The scripting functions could be tidied up to remove duplicates and inefficiency. The help file could be dragged into this decade. The installer could work better under Windows 7. The places files are put could be improved (eg. Documents folder).

Let's say I did that (or you did that). And let's say it took 100 hours. Probably twice that if the changes required documenting. That's a guess mind you. It might be 500 hours.

Now the question is, is that 100 to 500 hours better spent doing something else? Like maybe writing a new client from scratch using wxWidgets? Or making a fantastic mapper that works on every known MUD? Or writing a new client/server from scratch, maybe one that uses 3D graphics?

There are benefits to redoing the source, I can see. I probably won't be supporting it for the next 20 years, or even the next 10. So if it was rewritten to be easier to modify, that would be a help. But will people really be using MUSHclient in 10 years time? I am getting the feeling that with new consoles coming out (and even things like the iPhone/iPad with games on it) the market for MUD games will gradually dwindle.

Now it probably won't ever die, but could not MUSHclient in its current form, maybe with minor tweaks, not be used to service that "market" indefinitely?

Meanwhile, back to the scripting engine. Even making minor changes is likely to make existing scripts fail. I was worried that my change to the way the metatables are set up might even do that. Last month alone mushclient443.exe got 3424 hits on the web server, plus 685 hits for MUSHclient_4.43.zip. And those figures are generally consistent from month to month.

So, something like 4000 downloads a month, that's a lot of angry people if things change so that existing plugins don't work, or existing examples on this forum become invalid.

Here, let me show you something. I just logged into Aardwolf and typed their "clients" command, which shows how much each client is used based on its telnet identifications string.



I just want to point out that MUSHclient in its current form is the most popular one, easily. The second-most popular one is no longer being actively supported, particularly on Vista and Windows 7. Other clients, some of which have had more modern interfaces and a lot of work going into their design and implementation, have very low usage counts.

Once again, I am happy to make changes, or incorporate changes, that have some tangible end-player benefit. For example, rotating images, playing multiple sounds, playing background music, mappers, health bars, stuff like that. But I draw the line at changes for the sake of improvements to the code, where such changes may only benefit one or two people, those who actually read the source. And in fact, if major changes were made there it would simply make it harder for me to maintain the code, so that would actually be a negative change.

Alright then. I guess I'll get to work on Aspect.

Thank you for your patience with my clearly misguided notion of cleaning up the MUSHclient source. If you don't expect MUSHclient to be used for playing MUDs in the near future, then there's really no point, I have to admit. Meanwhile, I'm going to try to make MUDs more viable in the long-term.

By the way, I don't want to be picking at this scab or anything, but isn't that client-usage graph a little biased? Aardwolf offers MUSHclient as its "official" client.

While I'm at it... Yes, I like Lua a lot. No, I'm not cheating on it with Javascript. I've observed previously that we seem to have very conflicting views on source management, so there's no real surprise anywhere in this topic. But honestly, of the changes I've proposed, I think this one is the closest to the end-user and not a deep-in refactoring like t_regexp was.

For example, I've said before that it would be nice to pass functions instead of names to certain API functions, for example. If it were easier to bulk up the Lua glue interface, this would be possible without breaking backwards compatibility. Also, you can even "version" scripting interfaces, by giving the new one a new language identifier. If I were to, say, make extensive changes to the Lua interface, I'd probably make it a new interface and call it something else.

I know I make a lot of suggestions, but of late it's more to gauge interest in the idea, as I've mentioned before. I don't expect you to write anything just for me. If you were to ask me to get my hands dirty and implement it myself, I couldn't be any more willing! I just try to avoid it at first because, like you (for once), I don't want to expend effort when nobody's going to use it.

I chose Aardwolf because they make figures available. I said it was Aardwolf, and people who visit this forum know that I have been working with them to an extent.

However to compare more generally, there was a thread on the Mudstandards.org forum where various client authors disclosed their download figures.

[EDIT] (April 2015) Warning: Domain name mudstandards.org has been abandoned. That site is now an adult products shop.

According to that (rounded to nearest 100):

• MUSHclient: 4100
• zMUD: 3900
• cMUD: 2100
• TinTin+: 1600
• Mudlet: 1400

These figures are per month, and are historical (things may be different now) for around 2009-2010. They are self-disclosed, and hopefully cover every genre of MUDs, not just one that happens to recommend one client over another one.

Also, these are download figures. No-one knows really whether a program gets used once or is used daily for five years. Well, Zugg can tell you because of the clients hitting the web site to check for updates, so he is able to give you an execution count. There isn't much point in publishing that because I don't have comparative figures for other clients.

According to Zugg's figures, zMUD is trending down on execution counts, and cMUD is trending up, which you would expect because cMUD is newer.


Also see this page:

http://www.godwars2.org/mwi/clients

On that he lists client usage and MUSHclient is around 52% of usage, with no other client having more than 12% (again, that MUD probably favours MUSHclient because of the miniwindow stuff).

Good idea. I tried that once.

http://www.gammon.com.au/forum/?id=10043

http://www.gammon.com.au/forum/?id=10106

Respectfully, then, when you make a suggestion and other people don't show a lot of interest, is it necessary to get upset and effectively take your toys and leave? Enthusiasm is most certainly welcomed, however that doesn't mean that all ideas will be accepted as being a good priority at the moment. Don't forget that any change you make will take work on the part of other people; Nick at the least will have to review everything and become familiar with it, and in some instances it might even affect plug-in developers, and finally it might change user experience (hopefully for the better, of course). The point here is just that even if you were to do "all the work" in implementing something, there is always more work to be done in getting that something integrated into the main release.

Now, regarding this particular change. First you said it was about Javascript V8, now you say it's about (for example) functions rather than names for Lua. Well, if the goal is to make Lua support better, perhaps that could be addressed without a full rewrite of the entire scripting architecture?

One of the hardest things to avoid when getting into big projects like this is the temptation to rewrite lots of things just because you think it'll be "cleaner" in the end of the day. Sometimes it will, sometimes it won't but will seem so because you wrote it. Regardless in the end of the day there are usually more pressing things to take care of. Naturally this is a hard balance to strike because sometimes old code is crufty and causes more trouble than it's worth. But so far I don't think we've run into that point, at least not with demonstrable failures that have created clear costs.

I became discouraged. It seems like I'm thwarted at every turn with my attempts to work on the source. I've asked Nick before if there's anything specific I can do to help out. I can't find that discussion right now, but by and large there's no "official" feature/bug list for me to work on. Sans a Nick-blessed task, I figured I would clean up the code to make things easier to do.

(Besides, nobody wants my toys. I'm happy to share.)


I understand that Nick would have to become familiar with my changes, the point's been made previously. I don't understand what you mean by "a good priority" though. Like I explained, there's nothing else for me to do. But I don't really want to do nothing with MUSHclient, either. Aspect is a fine goal, but it's a big one, and I'm only one person.


I'm not sure whether you're trying to dissuade me from making changes, or explaining again that changes need to be carefully considered. Either way, Nick appears to be the only one allowed to change things.


I'm sorry, I never said it was about V8. That was an example of what could be done. Furthermore, my changes would allow both such things to be done more easily. I'm building a framework for features rather than piling more features right on top of old ones.

Personally, I can't consciously write code that's ugly or confusing. To implement either V8 or improved Lua support on top of the current state of affairs would be both.

But honestly... I'm just breaking CScriptEngine into two classes with a common interface. I'm still working on it, and it's not that hard to do. Nothing like what I expected to have to deal with.


Yes, I learned that lesson well from my 'old' branch in my GitHub repository.


I want to ask a honest question of you. Please don't take this the wrong way. How familiar are you with the MUSHclient source?

In my honest opinion, and meaning no slight to Nick himself, the whole thing is "crufty". I love MUSHclient to death, and it's a testament to his perseverance that it's so stable and functional, but it seems like such a tangled maze of knots and interdependencies that I can't begin to tell where to add something. The most I can do is understand what the code is doing and figure out how it could be made better, then figure out how to do something with it.

What about your miniwindow system?


I think the main point I have been trying to make is that features are more important than frameworks for features, and there are features that are needed -- like rich inventory list widgets, maps, and so forth.

Nick has very often talked about how nice it would be to have richer GUI environments, so even without having a whole framework for miniwindows, there is a lot of stuff that can be done.


Maybe it'll be easy and Nick won't mind integrating it. Your initial post made it sound not-so-easy. You said yourself you were trying to gauge interest; well, I'm just telling you that I don't think it's worth spending time on.


Probably not as much as you, because I haven't been spending so much time on it.


Well, in my experience, people sometimes get that feeling because they're confusing complexity for cruftiness. Something of this size will always be complex. You said yourself that Aspect is a big task; perhaps if you were to bring it to the level of stability and feature-richness that MUSHclient has, you would appreciate better how necessarily complex some stuff gets.

Even if it is "crufty", that is not in and of itself an argument for rewriting stuff. We've been over this before: there is a tradeoff to be made between time spent dealing with the consequences of "cruftiness" and time spent rewriting, fixing, documenting, testing, fixing some more, documenting more, etc., the changes to the code.

Heck, part of the reason that you get weird tangled mazes of knots and interdependencies is precisely that there can be subtle links between apparently different components. Obviously not all such cases are necessary, but how do you quickly tell which are which?

And in the end of the day, maybe Nick does prefer to spend his time writing rich graphical interfaces and methods for communication semantic information between server and client. Maybe he doesn't; I dunno. The point is just that it's his time to spend and I'm just trying to help you understand what you need to do to convince him that some task is worth undertaking. You sometimes act as if he never listens to you but that is demonstrably false if one looks at the suggestions forum; most recently he implemented the image rotation suggestion. Granted, he didn't implement everything you asked for, but he did take the suggestion and act upon it (and wrote the code, in this instance).

Still working on it off and on. Lua isn't the easiest language to do this in.


I disagree. The groundwork for a feature is just as important as the feature itself, if it's to be stable and agile. The miniwindow API itself is such a framework, and it's fairly hard to use for anything beyond a certain complexity. Orthogonality (I've said this before) would be a huge plus, but it's too late to change. MWAPI (my thing) is supposed to help, but I'm stymied by the API even here.

Granted, I haven't made many production plugins using miniwindows, but that's because it's hard.


The miniwindow API is the framework itself, and yes, there's a lot that can be done. Possibility is a very different thing from feasibility though.


It would be no easier than my t_regexp changes. If that's going nowhere, I'm not really going to hold my breath on the script engine changes. Pessimistic? Maybe.


There is a god object in the very core of the program. No sane person on Earth can call that good management of complexity. Everything, and I do mean everything, comes back to CMUSHclientDoc.


You start by not having public data fields unless necessary, and work out from there. Encapsulation is a Good Thing (tm), and I am emphatically not the only one to think so.


And I'm very grateful for that. It tends to come after a struggle, immediately after which I feel bad because I feel like I forced him into it. I just feel like I'm up against a brick wall when I'm the one doing the work.

Ask for things vs. do them yourself... gee, I honestly prefer the latter, but I seem to get my way more often with the former. That just makes me feel worse.

If you don't mind, I'm going to chronicle my progress on this, even if it never gets pulled into the source.

---

As with anything, I'm taking an iterative approach to this refactoring. I first tinkered with it in Notepad++ from scratch to get an idea of what to do, and then I started hacking around with the real deal to get a practical idea of the problem domain. Now I'm on to my third iteration, and hopefully my last. I believe I have a solid understanding of how to define IScriptEngine, so this iteration should turn out better than the last.

Just as CMUSHclientDoc implements an interface for the scripting engine to invoke, so IScriptEngine will define an interface for MUSHclient to invoke. Each "callback" style will have its own method defined in IScriptEngine, such as OnMXPClose and OnInstall. From an abstract perspective, these are really events that MUSHclient can notify an engine of. An engine is then free to react in any manner it wishes.

This is only part of the big picture, though. This will clean up the code, but it won't make it any easier to define a new engine that can use the CMUSHclientDoc scripting interface. This is because that interface is coupled very tightly with CMUSHclientDoc.

Once I've confirmed that the first refactoring works, I want to move the CMUSHclientDoc script interface out of CMUSHclientDoc, and relegate it to the same "layer" as the Lua glue functions. Effectively, the functionality would remain in CMUSHclientDoc, but the scripting "glue" is defined somewhere else, where it can be used specifically by the WSH engine.

With those two steps complete, I believe it should be far easier to manage, modify, and create scripting engines, and especially bind them more naturally to the client language. Fortunately I don't have to do them both at once, because I'm positive I'll need another set of iterations to go through with the script-interface refactoring. Still, I'm pretty confident in my battle plan thus far.

Well, either way, it is something to do that would provide clear and immediate benefit, and allow people to go forward more easily in a direction Nick has advocated many times.

But, well, yes, sometimes the truly useful features to end-users and developers will be difficult.


Might I suggest that it's complex because the task itself is complex?

Honestly I would be interested to see what a client would look like if you designed it from scratch, and how you would make everything so simple.


Nobody is disagreeing. People are only disagreeing about how necessary it is to go through big rewritings of things to encapsulate.


Well, I'd say that this is the case because in the former it just means that you sufficiently convinced him that it was a good idea that he implemented it himself, whereas in the latter he didn't do it himself usually because he didn't think it was sufficiently high on the priority list. (The priority list is w.r.t. time in general, not just a MUSHclient priority list.)

It's largely Lua that's making this hard on me; I think in objects, and Lua makes you build said objects from scratch. I honestly think it would be much easier to do in something like Ruby.


I'll grant you that, okay. Graphics programming is never the easiest thing. But it's rather funny that, without me knowing it, my API gravitated towards what GDI already does.

With MUSHclient there seems to be an "only one chance" thing going on. In general, once something's in you can never ever replace it. Huh, actually that does explain the "would rather work on new features", because if you never go back and redo things with what you've learned, you don't really need the flexibility. This, for once, isn't a complaint; it's just an observation.


Yeah, wouldn't we all... Usually I have ideas and I never execute them 100%. I really hope Aspect turns out differently. I just need to hop back in at some point.

I know I'm like the new kid here. Nick's probably been programming for several decades longer than I have, and you're probably not far off either. But I don't think I'm wrong here. (Whether I'm right is a different matter.)


The "never improve old features" thing, right. At least now I understand it.



There you go: he's basically the only one allowed to make changes. That's not a bad thing. I just think that, if he doesn't want me to contribute in big ways, he make that clear so I can do something that will be more useful to everyone. (Like MWAPI.)

Well I wouldn't say "never". :-) However the bar is high, yes.

I like to think of it in dollar terms, even if the dollars are imaginary. How many dollars does it cost to change something vs. how many dollars would you save or make from the change?

FWIW, fighting the itch to go and rewrite crufty stuff is a big learning point for all programmers, myself included. I still fight it at work sometimes. Old code is old and crufty code is crufty, but it all works, and there's plenty of new stuff that needs to get written, even if it's sometimes boring or hard.

Of course, when the old stuff doesn't work, we fix it; if it's a big pain to deal with and causes clear confusion and trouble, we consider spending time to rewrite it. Perhaps caution is more evidently appropriate in this case, though, when a software mistake can potentially cost millions of dollars a day...


FWIW, and I don't mean to put words in Nick's mouth here, but I don't think he doesn't want you to contribute in big ways, he just wants to see clear justification before he invests the (probably considerable amount of) time to review and familiarize himself with large changes like this.

After all, he will be the one who has to be responsible for and otherwise answer for it in the long term, e.g. when you go to college or get a job and have less time to spend on this stuff.

But there's a certain point where there's too much effort spent fighting the cruft when you could refactor it and save everyone man-hours in the long run, which saves/makes you "imaginary dollars". I've asserted that the whole codebase is crufty, and it's hard to make changes. Of course, you've asserted that you don't change old code unless it's broken. I'm up against a pretty formidable wall, as you can see!


*sigh* Yeah, I understand. The baffling thing is that clear code is easy to maintain. There's no "I think" or "it might be" in that sentence. It flat-out is. There may be a cost in time taken to review the changes, and if Nick doesn't want to do that, that's his prerogative. But time is saved in the long run.

I don't think it's very realistic to say that a software project the size of MUSHclient would ever be "easy to maintain". There are many interdepedencies at a conceptual level simply because the program is complex.

Also, cleaning cruft saves time in the long run only if you actually revisit that cruft. If you have a piece of a program that is a black box and works as a black box, it can be absolutely horrid inside but it doesn't matter as long as you don't have to go look inside.

One need not look at all of the pieces to understand a single component, ideally.


In my experience - not as much as Nick's, but it's there - the problem is that there is no black box.

I never said that. I am constantly improving old features, for example the ability to keep hotspots when you create a miniwindow. I added PNG file loading/saving for images. Miniwindows can now be resized.

What I am resisting is "improving" old code (stuff no-one looks at except you and me) when the improvement is simply to look nicer and maybe make it easier to make some hypothetical change in the future.

One major problem with MUSHclient is that it was written using the COM (Component Object Model) when I didn't really understand it - 15 years ago. And, that part is rubbish, really. For example,:

• GetGlobalOption is part of the world document when it is irrelevant to what world is open.
  
• Ditto for utilities like Base64Encode, XML decoding, etc.
  
• GetSelectionStartColumn shouldn't be part of the world object - there should be a View object because you can have multiple views open. There are quite a few similar functions.
  
• Functions that affect internal notepads should really be based on a Notepad object (not the world object).
  
• There should be a Miniwindows object, so you can easily manipulate them. Ditto for hotspots.

The implementation is truly a mess. I acknowledge that. Fortunately it works reasonably well in its current form.

But if you are going to improve it, all this should really be fixed up, big time. But then it absolutely won't be backwards compatible, as you can imagine. Scripts would need to be rewritten, all of them.

But since the topic you started is called "Scripting engine refactoring" I looked up what refactoring means, and this is what Wikipedia says:



Now when you start talking about adding the V8 Javascript engine I think you are straying from not modifying its behaviour, but lets assume that is quibbling, and that adding another script language is something we can currently easily do.

The core of my objection is that you are proposing to change the source code without modifying its external functional behavior.

http://en.wikipedia.org/wiki/Code_refactoring

Now sure, that page mentions the advantages of refactoring, and in the case of a large project (eg. for a financial organization) where changes are constantly required, this may well be a good thing.

But by the definition of refactoring, every hour that you and I spend on that is basically an hour that has no immediate benefit to anyone (because the functional behaviour is not modified). And it has to be carefully checked and tested.

Apart from mentioning Javascript you haven't suggested a major change that you want to make, that the un-refactored code makes impossible or difficult.

I've acknowledged above that the scripting interface is a mess, in many ways. But to fix it would render existing scripts (and all - or most of - the examples on this web site) unusable. Maybe you can see a benefit in that. I can't really. Not now.

Truly, my advice is, if you think the code is a mess, and the scripting interface is a mess, and miniwindows could be improved to suit the graphical API you are talking about then start again. Honestly. And I don't mean that in a rejecting way. I proposed starting again myself ...

http://www.gammon.com.au/forum/?id=8132

It could be done so much better if you start from scratch. Ask the Mudlet guys, they wrote a client recently.

Actually, speaking of them, I got an email from one of their developers who I hope won't mind being quoted here ...



I want you to sit and read that four or five times and digest what he said. Read the bit about "not being able to finish the project" if he tried to integrate lots of scripting languages - something you propose. Read the bit about "wasting time and resources on support for multiple other scripting languages".

I suggested a couple of years ago writing another client which used Lua only and was shouted down. Well, Mudlet did that and are thankful they did.

So what I am saying is, and what he said is, that changes to the scripting engine to support multiple languages would be wasting time and resources. I can't make it much clearer than that. I'm not saying it can't be done. Maybe someone would use it one day. Whether it is a good use of time is what is debatable.

Well, I'm not adding V8. I'm refactoring the code so it will be possible to add V8 (among other things).


Here's two bullets from your own list of things-that-could-be-improved:

Script-wise, these things are possible if we define a new interface. One that still uses Lua, yes, but one with a "thicker" layer of glue. And since it's implemented separately, plugins that use the older interface aren't affected.


That won't be necessary. The script engine interface is the lynchpin between scripts and MUSHclient. By allowing new interfaces to be defined separately from the old ones, old scripts need not be updated. It's another form of versioning.


I tried Mudlet, and I didn't really like it. Personal preference, I suppose.

I want to work on Aspect, but I fear that I don't have enough experience in the client field in general. MUSHclient is an already working codebase. I've mentioned this before, but since I'm learning from it, I wanted to give back to it too.


No, see, I agree. One language makes much more sense. I'm just working with what we have here, and trying to make it better. Expanding the interface support will allow us to craft a newer Lua interface without breaking older scripts, as I described earlier. (Among other things. I don't expect you to include a V8 interface, but I'd write it into my version of the source just because that's awesome.)


Hence why I'm working with what we've got, rather than angering your valued user base. :)


Fact is, it already supports multiple languages. It's also very rigid, and there are a lot of things that a newer Lua interface can offer that plugin authors would love.

Yes, I'm settling on an updated Lua interface as my "end goal", following these refactorings. I think it's a worthy goal, don't you?


As always, thanks for your honest feedback, Nick.

Having a thicker layer of glue for Lua does not require a full rewrite of the scripting engine to add support for any abstract scripting language. In fact, it seems that it would be far more expedient to not do that, and instead introduce tight coupling for Lua. You've just said yourself that having one language is fine and even better; why are we so concerned, then, with all these other hypothetical languages?

If the task at hand is "make more stuff possible for Lua", that gives us a very different direction than the (hypothetical) task of "add an abstract interface for integration with any scripting language".


This is not meant to offend in any way whatsoever, but to be an earnest question. If you don't feel that you have the experience to make it work, why do you feel that you know that all these proposed changes will actually be beneficial or even possible once all necessary interdependencies are sorted out?

With the current state of affairs, changing the Lua glue would break backwards compatibility with older scripts. Neither Nick or I are willing to sacrifice that. And I am emphatically not going to add another short-circuit behavior for an extended Lua interface like what was done originally for Lua. Heavier glue means more complex glue. It would be a veritable nightmare, not just to maintain, but indeed to write.


Things already work in MUSHclient. I can study it and understand it. With Aspect, I'm working from a blank slate. I'm sure I could do it in time, but I'd like to understand how a functional (and popular) client works instead, to save me the trouble. Sort of an inversion of the NIH syndrome. And once I understand how the pieces of MUSHclient work, I'm in a position to give back to the client.

Why? If you're only adding new ways of talking to Lua, any old script will work.


My experience doesn't agree with this, but eh. A magic scripting interface won't solve all of your problems; you will need the "heavier glue" one way or the other. In fact, your proposal makes it all the more complicated, because it has to be language-agnostic until the last minute...

Meh. If we're going to improve the Lua interface, I'd like to do it right the whole way down. Much more object-based, like Nick suggested with Miniwindow and Hotspot and Notepad objects.


My plan is to define a method in IScriptEngine for each MUSHclient-to-script call (i.e. alias callback, trigger callback, each plugin callback). It's language-agnostic right until the last minute, because each script engine decides how to handle that event's particular arguments.

I invite you to glance over how it's currently done, by the way. It's in the scripting/ folder, under scripting.h, scriptengine.cpp (WSH interface) and lua_scripting.cpp (Lua interface).

This isn't answering the question. What prevents you from having a full object interface without affecting the existing interface?

You could even create some kind of Lua object that had an entry per supported plugin callback, with the difference that the parameters would be richer userdata rather than strings etc. So it wouldn't even look that different as far as script authors are concerned.

I get the impression that you're going straight to the most complex solution but without even really considering the simpler solutions or what the practical tasks and goals are.


I'll take a look sometime soon, then.

More OO isn't the only thing you can do under this setup - it's just the most obvious thing. (Yes, I know I'm not suggesting any other things. I just don't really feel the need to prove my plan's worth, and I have to get to Karate.)


Hum. Can you explain this? A Lua object that had an entry per supported plugin callback... I'm not sure what that means.


Well, do keep in mind, I came at this from a refactoring angle. This was just one of the things you could theoretically do with such a refactoring. It wasn't really like "The Lua interface reeks in MUSHclient... lets make it better by revamping the whole scripting engine support!"

I also don't think it's the most complex solution, really. The actual operations of the current WSH and Lua methods won't change except with regards to the parameters each method takes. At the most basic level, I'm (1) splitting Lua and WSH code away from each other, and (2) splitting the myriad uses of CScriptEngine::Execute() into singular methods.

As an example, to make it easier when I was on my second iteration (i.e. working out the practical difficulties by diving into the code headfirst), I defined a private method Invoke() on the WSH interface, which took a DISPPARAMS reference just as the original Execute() did. The three new Execute methods - as I hadn't come up with the one-per-callback plan - took their arguments, stored them in a DISPPARAMS, and called Invoke() with it. It was trivial to take code calling Execute() and move the DISPPARAMS manipulation into an Execute method. What I left behind was singular and unified: it didn't care what scripting engine was involved, as long as it got the arguments.

(What really killed that second iteration is that the MXP error callback takes its arguments differently based on whether you're using Lua or WSH. My current plan solves that, and it's tidy to boot.)

But that's still not answering the question. :-(


Well, if the idea is to create some kind of rich object for interaction with MUSHclient, you could create some object that knew how to interact with the MUSHclient core, and you would add fields to the object for callbacks you want to support. The point here is just that there are alternatives to full scripting engine rewrites.


Sure. But refactoring should only very rarely be done for refactoring's sake; you should always have a clear, concrete task in mind.

What has come out of this discussion is that a concrete task would be improving Lua, because we all agreed that it's better to focus on one language rather than run around trying to support a whole bunch of them with rich integration. Your original angle might have been refactoring for refactoring's sake, but as you've said yourself, discussions change things, so we should talk about where we are now, not where things were at the beginning of the thread (unless of course you want to return to that, in which case we need to talk about that too).

I do think that my refactoring is the easiest way to accomplish this. *shrug* Now I have to run, I'll be back in a few hours.

One (perhaps frivolous) advantage to being able to create new interfaces is that you can build special-purpose interfaces. I note that the trigger/alias/timer filter dialogs let you define a filter, which is run through a one-shot CScriptEngine. You could define a ListFilterScriptEngine specifically for filtering, and define useful things there.

A specific example is how you have example_filters.txt distributed with MUSHclient. With a new filter script engine, you can include these into the global environment of the filter without disturbing the normal scripting environment, so a user would just do "filter = some_predefined_filter". You can also make filters easier to understand (and write) by changing this:


to this:


Filters are also relatively unused (and small!), so breaking changes are probably not a huge concern here. (If they are, you can just create a Lua state same as before, and add the new stuff on top of it. No big deal.)

Chronicling again:

--

I've just moved all of the Lua-specific code into LuaScriptEngine, and I've "flicked the switch" on which script engine is used:


MUSHclient runs fine and I'm still able to do scripting with it. None of the Execute refactoring has been done yet; so far I've simply separated the languages at the short-circuiting points in the original CScriptEngine. I have, on the other hand, moved CreateScriptEngine() into the constructor, and DisableScripting() into the destructor. A ScriptEngineException is thrown from the constructor if something goes wrong, and the destructor is hence automatically called. It also makes calling code a bit nicer, for example I'm happy with how the filtering code creates an engine:


You may notice I threw in an auto_ptr for the script engine. I just discovered those and they're really really useful, especially for RAII and destructing objects no matter what.


I wanted to keep things working while I refactored, so I left the Lua code in CScriptEngine while I did this. Now that I've "switched on" the LuaScriptEngine I can remove it, which is what I'll do next. After that I'll tackle refactoring Execute.

Huh... I discovered an odd assertion caused when deleting a CScriptEngine (in CMUSHclient::DisableScripting(), the 'delete m_ScriptEngine'), which I seem to have somehow introduced in one of my first refactoring changesets. I'm honestly baffled here, I can't figure out why it would suddenly start happening. The specific error is an invalid heap pointer, and it never even gets to the engine destructor. (It asserts in _CrtIsValidHeapPointer.)

I've isolated the weirdness to one commit [1], but I can't tell what could be the problem here. The issue only happens with a WSH script engine; the Lua engine is unaffected. The only odd thing I'm doing in that commit is adding some dynamic_casts to keep things working during the refactoring, but they only happen if it's a Lua engine... and they would cause an exception there, not when the engine is being released. What the humgii is going on here? :S

[1] http://github.com/Twisol/mushclient/commit/92496a4e2bc49b0cfe8c871c55b069b4ceade068

Casts "to keep things working" are a bit of a red flag. Even though the code which does this particular cast may not be executed in the problem case, the fact that a cast is required at all may point to the problem area.

Well, I want to ensure the code continues to compile after each step. Since I haven't refactored the Execute() methods yet, it still needs to access the L data member of CScriptEngine. Once the refactoring is complete, the scaffolding won't be necessary anyways.

At any rate... if I add a LuaState() method to IScriptEngine and hide L behind it - and thus remove the necessary dynamic_casts - it still asserts. =/

From what I can see from your design, you essentially have this:



Now if I compile that under g++ and Valgrind it, I get this:



So, the error message is correct, and your code needs looking at.

...Feh. I forgot the most basic of things: a virtual destructor. Destructing an object through its base pointer is just about guaranteed to fail otherwise. *sigh* That should have been obvious. Thank you so much Nick. :|

Perhaps this is an interesting case study of how seemingly innocuous changes can create subtle bugs that took a while to track down. Be happy that in this instance the cause was easy to find because it was so easily reproducible; had it been something even more subtle this could have cost even more. This is exactly the kind of stuff that makes Nick and I uneasy about this project: unless you have a clear goal in sight, why spend all this time tracking down problems you created for yourself?

Well, I'm not going to make up an excuse for what was definitely a stupid error on my part. I know that was a pretty novice mistake. All I can say is that I haven't done abstract bases in C++ before, so I hadn't worked with the technical details before; I left C++ soon after I had a passable understanding of OOP (which would be after I tried to develop a Snipes clone from scratch).

This is also why I'm keeping an audit trail of my changes with Git. I've taken your advice and collapsed related changes into one commit, which does help (even if it makes me uneasy). I was able to easily isolate the issue - I simply lacked a key point of knowledge. (I couldn't tell you what it was about Nick's post that did it, though. R-mode brain [*1] to the rescue, I guess! :P)


[*1] I'm reading "Pragmatic Thinking & Learning" from The Pragmatic Programmers. Really good book. By the way, have you heard of the broken windows hypothesis?

Given
1. Inheriting code is never easy.
2. MC wasn't always open source and had peer review or consistent coding standards.
3. Nick has said that he doesn't really see himself still supporting MC in 5 years.
4. and Paraphrasing him "Knowing what I know now, I'd start over" -- and the problem is that no one else knows what he knows unless they actually dive into his code.

I think it's cool that Twisol's taking the time and effort to understand Nick's code as a whole. The newer 'well-written' and the older 'spaghetti' code.

Where/when did he say this? I thought that au contraire, one reason Nick is reticent to include changes is that he'd have to invest time in fully understanding them all so that he'd be able to support them later.


No disagreement there. Taking the time to understand code is not what I've been objecting to.

I said that (page 1 of this thread). Whether you read into that, that I won't be supporting it in 5 years ... well it could be read that way. I am providing free support - I am not claiming that will go on indefinitely, nor am I giving notice that I will stop supporting it on any particular date. On such a date, whenever that is, it would be handy to have other people able to at the very least answer questions, and preferably make bug fixes and add enhancements.

That is one of the reasons the source was made openly available, so that the potential exists for others to support it. Unfortunately you also need the MFC libraries which limits support to people who have purchased those.

This is one of the reasons why I suggested that, if many hours are to go into improving the code, those hours might be better spent writing a new client from scratch, that doesn't depend on proprietary libraries.

In any case, congratulations Twisol on spotting the actual problem with your assertion. Interestingly, that slowed you down a couple of days, and there is regrettably no way of knowing how many other such cases will arise, with open-ended time-frames to solve.

The ideal case would be that you promptly, efficiently and without bugs refactor the code so it is much simpler, but continues to work with existing triggers, plugins etc.

One last point though, I have had people on this forum before who have been highly enthusiastic, made great suggestions, and in many ways contributed to the client.

Then after three to five years they get a job, move overseas, get married, have children ... something. Maybe they just lose interest. Then we never hear from them again.

If the code is changed in major ways, so that I no longer can understand it, and also the person who makes such changes goes on to greater things (maybe they become a 3D programmer for the newest MMO game!) then no-one knows how to support the client. Such a scenario might arise in the next five years, for example.

So in this situation there is a difference between people understanding the existing code (then either they or I can support it in five years) or changing it so that only they can support it.

Actually, I bought two new games from Steam, and I've been playing them when I need to mull things over. A bigger roadblock caused a correspondingly bigger delay. (And they're fun, too!)


That is truly my goal. So long as I keep the public API unchanged, all I need to do is keep myself from introducing odd bugs. Last night I fixed an issue caused by my lack of total understanding of auto_ptr: I was initializing it explicitly with NULL, which apparently is bad. Creating one via the default constructor does the same thing, and doesn't cause a crash. :S

I think the lesson here is to be extremely careful with building blocks I've never used before.

The latter case is more of a fork, because I know you won't pull in changes you can't support. ;) Hence, the onus is on me to produce code you can support, else my work will go to waste.

I might also add that I think the codebase is currently leaning slightly towards the "only you can support it" side of things. :)

[EDIT]: Of course, I need to know what changes you can support, and I can only know that if you're willing to give feedback on a changeset when it's ready for review.

Okay, I've gone through this iteration a ways, and I'm hitting a few bumps that aren't issues with just CScriptEngine alone. CallPlugin, for example: I'm not honestly sure how the arguments would be passed in. I could define some kind of union type - MUSHScriptVar? - and have a list of those passed in, and that would do it, but that would also cut off the lua_xmove() technique CallScript currently uses between two Lua stacks.

Anyways, I think I'm going to take a breather right now, and look at other parts of the code. I'm not used to working so long on a single task, heh. At least I learned a lot about how the scripting internals work!