We already have a create functions in the Account manager, wrapped by the utils.create.account function - the Account.create is a game-specific wrapper on top of that which confuses the matter; which one should one use?

Unfortunately it depends. Whether you use Account.objects.create() or calling utils.create_account(), both are low-level calls-- you ask for an account, you get one, nothing more. It does not perform the post-creation steps currently implemented only in CmdUnconnectedCreate.

As you point out, the benefit of implementing this as a class method is that if one decides they want additional steps taken (like subscribing to additional channels or granting starter bonuses) they can extend the functionality instead of having to reimplement all of it.

Linking back to the first negative point - this breaks consistency with other typeclasses. You can't do Object.create or Script.create, for example - the Account class becomes a discrepancy. This inconsistency does not an acceptable API do.

We could though. I just hadn't thought of that :)

Consolidate all typeclasses to have a .create method on themselves, in the simplest form only as a wrapper of the manager method. This is a big or small API change depending on if we then consider utils.create deprecated or not.

I think this is the best option, but we don't really need to deprecate utils.create at all. Leave that for power users who don't want any hand-holding, while implementing friendlier and more thorough object.create() methods for the sake of easier adoption and configuration. My opinion (and part of the basis for this PR) is that a naive dev should be able to call something like object.create() and receive a default feature-complete object where possible, unless overridden or extended to do otherwise.

@strikaco After some thinking I agree that this option is the sanest one - I think the way to go would be to add a new .create method on the Typeclass base (defaulting it to be a simple wrapper for the utils.create functions) and then override it in the subclasses where it's needed (like for Account.create in this case).

@strikaco After some thinking I agree that this option is the sanest one - I think the way to go would be to add a new .create method on the Typeclass base

For additional consideration in support of this course of action, note that these objects already had delete() methods implemented directly on themselves. It seems logical that create() should also be accessible in this manner.

(defaulting it to be a simple wrapper for the utils.create functions)

Looking into it, I notice that DefaultObjects are affected by the same workflow issue that led to this PR for Account objects. CmdCreate implements useful post-creation functionality (like setting locks and default descriptions) that is only available in-game through CmdCreate. I'm sure I'll find the same when I get to Character/Room/Exit objects too.

Wouldn't it make more sense to migrate the effective functionality of CmdCreateWhatever to WhateverObject.create() across the board instead of making them simple wrappers? I think you alluded to this with "in the simplest form only as a wrapper of the manager method" but want to make sure we're on the same page. This would be more consistent in application (not just an exception for Accounts) and much easier for newcomers to grasp.

@Griatch Whenever you get a moment, I'd appreciate a quick review. Thanks!

How many typeclassed entities are there? I see "create_object", "create_script", "create_help_entry", "create_message", "create_channel", "create_account" listed in utils.create(). So far it seems I'm overlooking help_entry, message and channel?

What should I have the typeclass base do? I can't account for custom entities or their creation path. Should I just raise a not-implemented exception?

The base typeclasses are DefaultAccount, DefaultObject, DefaultScript and DefaultChannel.

I mis-spoke about the Msg, this is not a typeclass, just an idmapped entity. Same with HelpEntry (although I have toyed with the idea making help entries typeclassed, they are not at this time).

While the TypeclassBase could forward its .create to cls.objects.create, I think it's probably best to not touch it further now that I look at it - keep the create functions only on the parent typeclasses above.

I believe I've addressed the feedback to date; whenever you get a moment this is ready for review.

@strikaco The code changes look good, but you must test your PRs against the entire test suite - it appears to pass when you run your tests in isolation, but fail when ran in combination with the rest of the Evennia test suite (and I've compared with and without this branch - the fault is introduced here). See the results from the Travis integration.
The likely cause of this is because you are making assumptions about the state of your test environment what is not valid when run with other tests. In principle unit tests should be isolated enough that this should not happen (so the problem likely lies in another test that runs before this), but one way or another - the test suite does not pass.

@Griatch All bugs should be squashed; comprehensive tests pass locally but it doesn't look like Travis picked up the last commit. What now?

...Looks like it picked it up after all. This one's ready.